Repository: Yoctol/bottender
Pull request #364 (2018-11-09 05:04:06)

implement skipProfile messenger
If the user profile is not necessary in the bot responding logic, we can skip the profile updating and have a better responding experience.

With this flag setted true, skiping all the `getUserProfile` api calls when session updating, providing better performance when dealing heavy traffic.

codecov-io: # [Codecov](https://codecov.io/gh/Yoctol/bottender/pull/364?src=pr&el=h1) Report
> Merging [#364](https://codecov.io/gh/Yoctol/bottender/pull/364?src=pr&el=desc) into [master](https://codecov.io/gh/Yoctol/bottender/commit/4a75f44f98f44e37ee47c3f5fe33838a009e25d2?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/Yoctol/bottender/pull/364/graphs/tree.svg?width=650&token=JX3EGtxYH3&height=150&src=pr)](https://codecov.io/gh/Yoctol/bottender/pull/364?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #364      +/-   ##
==========================================
+ Coverage   89.88%   89.89%   +<.01%     
==========================================
  Files         116      116              
  Lines        3450     3453       +3     
  Branches      780      782       +2     
==========================================
+ Hits         3101     3104       +3     
  Misses        309      309              
  Partials       40       40
```


| [Impacted Files](https://codecov.io/gh/Yoctol/bottender/pull/364?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/bot/MessengerConnector.js](https://codecov.io/gh/Yoctol/bottender/pull/364/diff?src=pr&el=tree#diff-c3JjL2JvdC9NZXNzZW5nZXJDb25uZWN0b3IuanM=) | `91.2% <100%> (+0.29%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Yoctol/bottender/pull/364?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Yoctol/bottender/pull/364?src=pr&el=footer). Last update [4a75f44...cbdd2fb](https://codecov.io/gh/Yoctol/bottender/pull/364?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/bot/MessengerConnector.js b/src/bot/MessengerConnector.js
index 41bdf2a3..b0ae6673 100644
--- a/src/bot/MessengerConnector.js
+++ b/src/bot/MessengerConnector.js
@@ -84,6 +84,7 @@ type ConstructorOptions = {|
   batchConfig?: ?Object,
   origin?: string,
   skipAppSecretProof?: ?boolean,
+  skipProfile?: ?boolean,
 |};
 
 export default class MessengerConnector
@@ -94,6 +95,8 @@ export default class MessengerConnector
 
   _appSecret: string;
 
+  _skipProfile: boolean;
+
   _mapPageToAccessToken: ?(pageId: string) => ?Promise<string>;
 
   _verifyToken: ?string;
@@ -112,6 +115,7 @@ export default class MessengerConnector
     batchConfig,
     origin,
     skipAppSecretProof,
+    skipProfile,
   }: ConstructorOptions) {
     this._client =
       client ||
@@ -128,6 +132,9 @@ export default class MessengerConnector
     this._mapPageToAccessToken = mapPageToAccessToken;
     this._verifyToken = verifyToken;
 
+    // FIXME: maybe set this default value as true
+    this._skipProfile = typeof skipProfile === 'boolean' ? skipProfile : false;
+
     this._batchConfig = batchConfig || null;
     if (this._batchConfig) {
       this._batchQueue = new MessengerBatchQueue(
@@ -255,24 +262,31 @@ export default class MessengerConnector
       }
 
       // FIXME: refine user
-      let user = {};
-      try {
-        user = await this._client.getUserProfile(senderId, {
-          access_token: customAccessToken,
-        });
-      } catch (e) {
-        warning(
-          false,
-          'getUserProfile() failed, `session.user` will only have `id`'
-        );
-        console.error(e);
-      }
+      if (this._skipProfile) {
+        session.user = {
+          _updatedAt: new Date().toISOString(),
+          id: senderId,
+        };
+      } else {
+        let user = {};
+        try {
+          user = await this._client.getUserProfile(senderId, {
+            access_token: customAccessToken,
+          });
+        } catch (e) {
+          warning(
+            false,
+            'getUserProfile() failed, `session.user` will only have `id`'
+          );
+          console.error(e);
+        }
 
-      session.user = {
-        _updatedAt: new Date().toISOString(),
-        ...user,
-        id: senderId,
-      };
+        session.user = {
+          _updatedAt: new Date().toISOString(),
+          ...user,
+          id: senderId,
+        };
+      }
     }
 
     // TODO: remove later


Test patch:
diff --git a/src/bot/__tests__/MessengerConnector.spec.js b/src/bot/__tests__/MessengerConnector.spec.js
index eda1509d..2160d3b1 100644
--- a/src/bot/__tests__/MessengerConnector.spec.js
+++ b/src/bot/__tests__/MessengerConnector.spec.js
@@ -164,11 +164,12 @@ const webhookTestRequest = {
 };
 
 function setup(
-  { accessToken, appSecret, mapPageToAccessToken, verifyToken } = {
+  { accessToken, appSecret, mapPageToAccessToken, verifyToken, skipProfile } = {
     accessToken: ACCESS_TOKEN,
     appSecret: APP_SECRET,
     mapPageToAccessToken: jest.fn(),
     verifyToken: undefined,
+    skipProfile: false,
   }
 ) {
   const mockGraphAPIClient = {
@@ -183,6 +184,7 @@ function setup(
       appSecret,
       mapPageToAccessToken,
       verifyToken,
+      skipProfile,
     }),
   };
 }
@@ -423,6 +425,23 @@ describe('#updateSession', () => {
     );
     expect(console.error).toBeCalledWith(error);
   });
+
+  it(`update session without gettiing user's profile when skipProfile setted true`, async () => {
+    const { connector, mockGraphAPIClient } = setup({
+      skipProfile: true,
+    });
+
+    const session = {};
+    await connector.updateSession(session, request.body);
+
+    expect(mockGraphAPIClient.getUserProfile).not.toBeCalled();
+    expect(session).toEqual({
+      user: {
+        _updatedAt: expect.any(String),
+        id: '1412611362105802',
+      },
+    });
+  });
 });
 
 describe('#mapRequestToEvents', () => {