Users/ersuo/fix message order (#46)

* use conversation from proxy

* add lwi and chatid to telemetry property

* early return

* fixing UT and some constant's name

* skip ack if polling already received receipt message

* changing ack map to sending map

* remove testing method

* remove testing method

* remove comment

__tests__/ic3/enhancers/egress/createEgressMessageActivityMiddleware.spec.js

@@ -1,3 +1,7 @@
+import * as sendingMessageClass from './../../../../src/utils/sendingMessageMap';
+
+import createUserMessageToDirectLineActivityMapper, {testExportFromCreateUserMessageToDirectLineActivityMapper} from "./../../../../src/ic3/enhancers/ingress/mappers/createUserMessageToDirectLineActivityMapper";
+
 import { ActivityType } from './../../../../src/types/DirectLineTypes';
 import { StateKey } from './../../../../src/types/ic3/IC3AdapterState';
 import { TelemetryEvents } from './../../../../src/types/ic3/TelemetryEvents';
@@ -162,6 +166,131 @@ describe('test createEgressMessageActivityMiddleware', () => {
         expect(sendMessageMock).toHaveBeenCalledWith(expectedMessage);
     });
 
+    it("should ack message if status 201 returned", async () => {
+        const clientActivityId = 'client_activity_id:1'
+        const activity = {
+            type: ActivityType.Message,
+            channelData: {
+                clientActivityID: testActivityId,
+                tags: [clientActivityId, 'testTag']
+            },
+            from: { id: 'testFromId' },
+            timestamp: 123,
+            value: {}
+        };
+        const fakeGetState = (key) => {
+            switch(key) {
+                case StateKey.Conversation: {
+                    return {
+                        sendMessage: () => {
+                            console.log("fake send message called");
+                            return Promise.resolve({
+                                status: 201,
+                                contextid: "4308e7a8-4acc-4ff3-92dd-36eee65ff987",
+                                clientmessageid: "609789f4-78ca-4e30-9703-af03619f8940"
+                            })
+                        },
+                        sendFileMessage: sendFileMessageMock
+                    }
+                }
+                case StateKey.UserDisplayName: return StateKey.UserDisplayName;
+                case StateKey.Logger: return { logClientSdkTelemetryEvent: logClientSdkTelemetryEventSpy};
+                case StateKey.ChatId: return "1234";
+                default: return null
+            }
+        }
+
+        const fakeIngress = jest.fn();
+
+        await createEgressMessageActivityMiddleware()({getState: fakeGetState, ingress: fakeIngress})(next)(activity);
+        expect(fakeIngress).toHaveBeenCalled();
+        sendingMessageClass.clearAll();
+    });
+
+
+    it("should not ack message if status 201 not returned", async () => {
+        const clientActivityId = 'client_activity_id:1'
+        const activity = {
+            type: ActivityType.Message,
+            channelData: {
+                clientActivityID: testActivityId,
+                tags: [clientActivityId, 'testTag']
+            },
+            from: { id: 'testFromId' },
+            timestamp: 123,
+            value: {}
+        };
+        const fakeGetState = (key) => {
+            switch(key) {
+                case StateKey.Conversation: {
+                    return {
+                        sendMessage: () => {
+                            console.log("fake send message called");
+                            return Promise.resolve({
+                                status: 400,
+                                contextid: "4308e7a8-4acc-4ff3-92dd-36eee65ff987",
+                                clientmessageid: "609789f4-78ca-4e30-9703-af03619f8940"
+                            })
+                        },
+                        sendFileMessage: sendFileMessageMock
+                    }
+                }
+                case StateKey.UserDisplayName: return StateKey.UserDisplayName;
+                case StateKey.Logger: return { logClientSdkTelemetryEvent: logClientSdkTelemetryEventSpy};
+                case StateKey.ChatId: return "1234";
+                default: return null
+            }
+        }
+
+        const fakeIngress = jest.fn();
+
+        await createEgressMessageActivityMiddleware()({getState: fakeGetState, ingress: fakeIngress})(next)(activity);
+        expect(fakeIngress).not.toHaveBeenCalled();
+    });
+
+    it("should not ack message if message is not in the sending map", async () => {
+        const clientActivityId = 'client_activity_id:1'
+        const activity = {
+            type: ActivityType.Message,
+            channelData: {
+                clientActivityID: testActivityId,
+                tags: [clientActivityId, 'testTag']
+            },
+            from: { id: 'testFromId' },
+            timestamp: 123,
+            value: {}
+        };
+        const fakeGetState = (key) => {
+            switch(key) {
+                case StateKey.Conversation: {
+                    return {
+                        sendMessage: () => {
+                            console.log("fake send message called");
+                            return Promise.resolve({
+                                status: 201,
+                                contextid: "4308e7a8-4acc-4ff3-92dd-36eee65ff987",
+                                clientmessageid: "609789f4-78ca-4e30-9703-af03619f8940"
+                            })
+                        },
+                        sendFileMessage: sendFileMessageMock
+                    }
+                }
+                case StateKey.UserDisplayName: return StateKey.UserDisplayName;
+                case StateKey.Logger: return { logClientSdkTelemetryEvent: logClientSdkTelemetryEventSpy};
+                case StateKey.ChatId: return "1234";
+                default: return null
+            }
+        }
+
+        const fakeIngress = jest.fn();
+        const originalIsStillSending = sendingMessageClass.isStillSending;
+        sendingMessageClass.isStillSending = (messageId) => {console.log("still sending returning false"); return false};
+        await createEgressMessageActivityMiddleware()({getState: fakeGetState, ingress: fakeIngress})(next)(activity);
+        expect(fakeIngress).not.toHaveBeenCalled();
+        sendingMessageClass.clearAll();
+        sendingMessageClass.isStillSending = originalIsStillSending;
+    });
+
     it('should send correct file message', async () => {
         const clientActivityId = 'client_activity_id:1'
         const activity = {

__tests__/ic3/utils/ackedMessageMap.spec.js

@@ -0,0 +1,30 @@
+import * as sendingMessageMapClass from './../../../src/utils/sendingMessageMap.ts'
+
+import uniqueId from './../../../src/ic3/utils/uniqueId';
+
+describe('sendingMessageMap test suite', () => {
+    test('clear the map if 5000 message inserted', () => {
+        for(let i = 0; i<=5000; i ++) {
+            sendingMessageMapClass.addToSendingMessageIdMap(uniqueId(), {});
+        }
+        expect(sendingMessageMapClass.size()).toBe(5001);
+        sendingMessageMapClass.addToSendingMessageIdMap(uniqueId(), {});
+        expect(sendingMessageMapClass.size()).toBe(1);
+    });
+
+    test('can successfully add message to the map', () => {
+        let uid = uniqueId();
+        sendingMessageMapClass.addToSendingMessageIdMap(uid, {});
+        let result = sendingMessageMapClass.isStillSending(uid);
+        expect(result).toBeTruthy();
+        sendingMessageMapClass.removeFromSendingMessageIdMap(uid);
+    });
+
+    test('can successfully verify a message not in the map', () => {
+        let uid = "25bd2632-788a-48ba-9ac8-a0a8b0ac9eb3";
+        sendingMessageMapClass.addToSendingMessageIdMap(uid, {});
+        let result = sendingMessageMapClass.isStillSending("88b40926-5d27-4dee-bce6-536c5ca563bd");
+        expect(result).toBeFalsy();
+        sendingMessageMapClass.removeFromSendingMessageIdMap(uid);
+    });
+});

src/ic3/enhancers/egress/createEgressMessageActivityMiddleware.ts

@@ -1,6 +1,7 @@
 /// <reference path="../../../types/ic3/external/Model.d.ts" />
 
 import { IC3AdapterState, StateKey } from '../../../types/ic3/IC3AdapterState';
+import { addToSendingMessageIdMap, getMessageFromSendingMap, isStillSending } from '../../../utils/sendingMessageMap';
 import { getAdapterContextDetails, stringifyHelper } from '../../../utils/logMessageFilter';
 
 import { ActivityType } from '../../../types/DirectLineTypes';
@@ -8,7 +9,6 @@ import { EgressMiddleware } from '../../../applyEgressMiddleware';
 import { IC3DirectLineActivity } from '../../../types/ic3/IC3DirectLineActivity';
 import { TelemetryEvents } from '../../../types/ic3/TelemetryEvents';
 import { Translated } from '../../Constants';
-import { addToMessageIdSet } from '../../../utils/ackedMessageSet';
 import { compose } from 'redux';
 import createTypingMessageToDirectLineActivityMapper from '../ingress/mappers/createThreadToDirectLineActivityMapper';
 import createUserMessageToDirectLineActivityMapper from '../ingress/mappers/createUserMessageToDirectLineActivityMapper';
@@ -116,6 +116,7 @@ export default function createEgressMessageActivityMiddleware(): EgressMiddlewar
         }
       );
     } else {
+      addToSendingMessageIdMap(message.clientmessageid, activity);
       const response = await conversation.sendMessage(message);
       getState(StateKey.Logger)?.logClientSdkTelemetryEvent(Microsoft.CRM.Omnichannel.IC3Client.Model.LogLevel.DEBUG,
         {
@@ -129,10 +130,13 @@ export default function createEgressMessageActivityMiddleware(): EgressMiddlewar
         }
       );
       if (ingress && response?.status === 201 && response?.contextid && response?.clientmessageid && !hasTargetTag(message, Translated)) {
-        const ackActivity:any = await convertMessage(message);
-        ackActivity.channelData.clientActivityID = activity.channelData.clientActivityID;
-        if (addToMessageIdSet) addToMessageIdSet(message.clientmessageid);
-        ingress(ackActivity);
+        /**
+         * set message to "sent" state only if polling has not fetched the message back yet
+         */
+        if (isStillSending(message.clientmessageid)) {
+          let ackActivity = getMessageFromSendingMap(message.clientmessageid);
+          ackActivity && ingress(ackActivity);
+        }
       }
     }
   };

src/ic3/enhancers/ingress/subscribeNewMessageAndThreadUpdate.ts

@@ -3,8 +3,8 @@
 import { AdapterEnhancer, ReadyState } from '../../../types/AdapterTypes';
 import { ConnectionStatusObserverWaitingTime, MissingAckFromPollingError, Reinitialize, ReloadAllMessageInterval, TranslationMessageTag } from '../../Constants';
 import { IC3AdapterState, StateKey } from '../../../types/ic3/IC3AdapterState';
-import { alreadyAcked, removeFromMessageIdSet } from '../../../utils/ackedMessageSet';
 import { extendError, logMessagefilter, stringifyHelper } from '../../../utils/logMessageFilter';
+import { isStillSending, removeFromSendingMessageIdMap } from '../../../utils/sendingMessageMap';
 
 import ConnectivityManager from '../../utils/ConnectivityManager';
 import { ConversationControllCallbackOnEvent } from '../../createAdapterEnhancer';
@@ -239,8 +239,9 @@ export default function createSubscribeNewMessageAndThreadUpdateEnhancer(): Adap
                       })
                     }
                   );
-                  if (alreadyAcked(message.clientmessageid)) {
-                    removeFromMessageIdSet(message.clientmessageid);
+                  // the receipt message had been successfully polled back, add to map to avoid further update on the activity status
+                  if (isStillSending(message.clientmessageid)) {
+                    removeFromSendingMessageIdMap(message.clientmessageid);
                   }
                   !unsubscribed && next(activity);
                 });

src/utils/ackedMessageSet.ts

@@ -1,15 +0,0 @@
-const messageIdSet = new Set<string>();
-
-export function addToMessageIdSet(clientmessageid: string) {
-    messageIdSet.add(clientmessageid);
-}
-
-export function removeFromMessageIdSet(clientmessageid: string) {
-    if (messageIdSet.has(clientmessageid)) {
-        messageIdSet.delete(clientmessageid);
-    }
-}
-
-export function alreadyAcked(clientmessageid: string) {
-    return messageIdSet.has(clientmessageid);
-}

src/utils/sendingMessageMap.ts

@@ -0,0 +1,38 @@
+const messageIdMap = new Map<string, any>();
+const MAX_CAPACITY = 5000;
+
+function checkAndCleanSendingMessageMap() {
+    if (messageIdMap.size > MAX_CAPACITY) {
+        clearAll();
+    }
+}
+
+export function addToSendingMessageIdMap(clientmessageid: string, activity: any) {
+    checkAndCleanSendingMessageMap();
+    messageIdMap.set(clientmessageid, activity);
+}
+
+export function removeFromSendingMessageIdMap(clientmessageid: string) {
+    if (isStillSending(clientmessageid)) {
+        messageIdMap.delete(clientmessageid);
+    }
+}
+
+export function getMessageFromSendingMap(clientmessageid: string) {
+    if (isStillSending(clientmessageid)) {
+        return messageIdMap.get(clientmessageid);
+    }
+    return null;
+}
+
+export function isStillSending(clientmessageid: string) {
+    return messageIdMap.has(clientmessageid);
+}
+
+export function size() {
+    return messageIdMap.size;
+}
+
+export function clearAll() {
+    messageIdMap.clear();
+}