From d7d764cf8862f2ac0bf3e387342f1dd88ebaac5a Mon Sep 17 00:00:00 2001 From: Josh Gummersall Date: Wed, 14 Oct 2020 12:47:10 -0700 Subject: [PATCH] Filter continueConversation events when logging Fixes #2805 --- libraries/botbuilder-core/package.json | 1 + .../botbuilder-core/src/transcriptLogger.ts | 44 +- .../tests/transcriptMiddleware.test.js | 428 +++++++++--------- .../botbuilder/src/botFrameworkAdapter.ts | 5 +- libraries/botframework-schema/src/index.ts | 13 +- 5 files changed, 245 insertions(+), 246 deletions(-) diff --git a/libraries/botbuilder-core/package.json b/libraries/botbuilder-core/package.json index cae3726a8b..1d7e71d3e0 100644 --- a/libraries/botbuilder-core/package.json +++ b/libraries/botbuilder-core/package.json @@ -30,6 +30,7 @@ "@types/node": "^10.17.27", "mocha": "^6.2.3", "nyc": "^15.1.0", + "sinon": "^7.5.0", "source-map-support": "^0.5.3", "ts-node": "^4.1.0", "typescript": "3.5.3", diff --git a/libraries/botbuilder-core/src/transcriptLogger.ts b/libraries/botbuilder-core/src/transcriptLogger.ts index a14cd903a2..a743c124ee 100644 --- a/libraries/botbuilder-core/src/transcriptLogger.ts +++ b/libraries/botbuilder-core/src/transcriptLogger.ts @@ -6,7 +6,13 @@ * Licensed under the MIT License. */ -import { Activity, ActivityTypes, ConversationReference, ResourceResponse } from 'botframework-schema'; +import { + Activity, + ActivityEventNames, + ActivityTypes, + ConversationReference, + ResourceResponse, +} from 'botframework-schema'; import { Middleware } from './middlewareSet'; import { TurnContext } from './turnContext'; @@ -47,12 +53,12 @@ export class TranscriptLoggerMiddleware implements Middleware { // hook up onSend pipeline context.onSendActivities( - async (ctx: TurnContext, activities: Partial[], next2: () => Promise) => { + async (ctx: TurnContext, activities: Partial[], next: () => Promise) => { // Run full pipeline. - const responses: ResourceResponse[] = await next2(); + const responses = await next(); - activities.map((a: Partial, index: number) => { - const clonedActivity = this.cloneActivity(a); + activities.forEach((activity, index) => { + const clonedActivity = this.cloneActivity(activity); clonedActivity.id = responses && responses[index] ? responses[index].id : clonedActivity.id; // For certain channels, a ResourceResponse with an id is not always sent to the bot. @@ -76,12 +82,12 @@ export class TranscriptLoggerMiddleware implements Middleware { ); // hook up update activity pipeline - context.onUpdateActivity(async (ctx: TurnContext, activity: Partial, next3: () => Promise) => { + context.onUpdateActivity(async (ctx: TurnContext, activity: Partial, next: () => Promise) => { // run full pipeline - const response: void = await next3(); + const response: void = await next(); // add Message Update activity - const updateActivity: Activity = this.cloneActivity(activity); + const updateActivity = this.cloneActivity(activity); updateActivity.type = ActivityTypes.MessageUpdate; this.logActivity(transcript, updateActivity); @@ -90,13 +96,13 @@ export class TranscriptLoggerMiddleware implements Middleware { // hook up delete activity pipeline context.onDeleteActivity( - async (ctx: TurnContext, reference: Partial, next4: () => Promise) => { + async (ctx: TurnContext, reference: Partial, next: () => Promise) => { // run full pipeline - await next4(); + await next(); // add MessageDelete activity // log as MessageDelete activity - const deleteActivity: Partial = TurnContext.applyConversationReference( + const deleteActivity = TurnContext.applyConversationReference( { type: ActivityTypes.MessageDelete, id: reference.activityId, @@ -105,7 +111,7 @@ export class TranscriptLoggerMiddleware implements Middleware { false ); - this.logActivity(transcript, deleteActivity); + this.logActivity(transcript, this.cloneActivity(deleteActivity)); } ); @@ -113,21 +119,20 @@ export class TranscriptLoggerMiddleware implements Middleware { await next(); // flush transcript at end of turn - while (transcript.length > 0) { + while (transcript.length) { try { - const activity: Activity = transcript.shift(); // If the implementation of this.logger.logActivity() is asynchronous, we don't // await it as to not block processing of activities. // Because TranscriptLogger.logActivity() returns void or Promise, we capture // the result and see if it is a Promise. - const logActivityResult = this.logger.logActivity(activity); + const maybePromise = this.logger.logActivity(transcript.shift()); // If this.logger.logActivity() returns a Promise, a catch is added in case there // is no innate error handling in the method. This catch prevents // UnhandledPromiseRejectionWarnings from being thrown and prints the error to the // console. - if (logActivityResult instanceof Promise) { - logActivityResult.catch((err) => { + if (maybePromise instanceof Promise) { + maybePromise.catch((err) => { this.transcriptLoggerErrorHandler(err); }); } @@ -146,7 +151,10 @@ export class TranscriptLoggerMiddleware implements Middleware { activity.timestamp = new Date(); } - transcript.push(activity); + // We should not log ContinueConversation events used by skills to initialize the middleware. + if (!(activity.type === ActivityTypes.Event && activity.name === ActivityEventNames.ContinueConversation)) { + transcript.push(activity); + } } /** diff --git a/libraries/botbuilder-core/tests/transcriptMiddleware.test.js b/libraries/botbuilder-core/tests/transcriptMiddleware.test.js index 0a26138e32..493d549e34 100644 --- a/libraries/botbuilder-core/tests/transcriptMiddleware.test.js +++ b/libraries/botbuilder-core/tests/transcriptMiddleware.test.js @@ -1,78 +1,58 @@ const assert = require('assert'); -const { TestAdapter, TranscriptLoggerMiddleware, MemoryTranscriptStore, ActivityTypes } = require('../'); - -class SyncErrorLogger { - logActivity(activity) { - throw new Error(); - } -} - -class AsyncErrorLogger { - async logActivity(activity) { - throw new Error(); - } -} - -class SyncThrowerLogger { - logActivity(activity) { - throw 1; - } -} - -class AsyncThrowerLogger { - async logActivity(activity) { - throw 2; - } -} +const sinon = require('sinon'); +const { + ConsoleTranscriptLogger, + TestAdapter, + TranscriptLoggerMiddleware, + MemoryTranscriptStore, + ActivityTypes, + ActivityEventNames, +} = require('../'); describe(`TranscriptLoggerMiddleware`, function () { this.timeout(5000); - it(`should log activities`, function (done) { - var conversationId = null; - var transcriptStore = new MemoryTranscriptStore(); - var adapter = new TestAdapter(async (context) => { + it(`should log activities`, async function () { + let conversationId = null; + const transcriptStore = new MemoryTranscriptStore(); + const adapter = new TestAdapter(async (context) => { conversationId = context.activity.conversation.id; var typingActivity = { type: ActivityTypes.Typing, - relatesTo: context.activity.relatesTo + relatesTo: context.activity.relatesTo, }; await context.sendActivity(typingActivity); await context.sendActivity(`echo:${context.activity.text}`); - }).use(new TranscriptLoggerMiddleware(transcriptStore)); - adapter + await adapter .send('foo') - .assertReply(activity => assert.equal(activity.type, ActivityTypes.Typing)) + .assertReply((activity) => assert.equal(activity.type, ActivityTypes.Typing)) .assertReply('echo:foo') .send('bar') - .assertReply(activity => assert.equal(activity.type, ActivityTypes.Typing)) - .assertReply('echo:bar') - .then(() => { - transcriptStore.getTranscriptActivities('test', conversationId).then(pagedResult => { - assert.equal(pagedResult.items.length, 6); - assert.equal(pagedResult.items[0].text, 'foo'); - assert.equal(pagedResult.items[1].type, ActivityTypes.Typing); - assert.equal(pagedResult.items[2].text, 'echo:foo'); - assert.equal(pagedResult.items[3].text, 'bar'); - assert.equal(pagedResult.items[4].type, ActivityTypes.Typing); - assert.equal(pagedResult.items[5].text, 'echo:bar'); - pagedResult.items.forEach(a => { - assert(a.timestamp); - }) - done(); - }); - }); + .assertReply((activity) => assert.equal(activity.type, ActivityTypes.Typing)) + .assertReply('echo:bar'); + + const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId); + assert.equal(pagedResult.items.length, 6); + assert.equal(pagedResult.items[0].text, 'foo'); + assert.equal(pagedResult.items[1].type, ActivityTypes.Typing); + assert.equal(pagedResult.items[2].text, 'echo:foo'); + assert.equal(pagedResult.items[3].text, 'bar'); + assert.equal(pagedResult.items[4].type, ActivityTypes.Typing); + assert.equal(pagedResult.items[5].text, 'echo:bar'); + pagedResult.items.forEach((a) => { + assert(a.timestamp); + }); }); - it(`should log update activities`, function (done) { - var conversationId = null; + it(`should log update activities`, async function () { + let conversationId = null; let activityToUpdate = null; - var transcriptStore = new MemoryTranscriptStore(); - var adapter = new TestAdapter(async (context) => { + const transcriptStore = new MemoryTranscriptStore(); + const adapter = new TestAdapter(async (context) => { conversationId = context.activity.conversation.id; if (context.activity.text === 'update') { activityToUpdate.text = 'new response'; @@ -85,55 +65,58 @@ describe(`TranscriptLoggerMiddleware`, function () { // clone the activity, so we can use it to do an update activityToUpdate = JSON.parse(JSON.stringify(activity)); } - }).use(new TranscriptLoggerMiddleware(transcriptStore)); - adapter - .send('foo') - .delay(100) - .send('update') - .delay(100) - .then(() => { - transcriptStore.getTranscriptActivities('test', conversationId).then(pagedResult => { - assert(pagedResult.items.length, 4); - assert(pagedResult.items[0].text, 'foo'); - assert(pagedResult.items[1].text, 'response'); - assert(pagedResult.items[2].text, 'new response'); - assert(pagedResult.items[3].text, 'update'); - done(); - }); - }); + await adapter.send('foo').delay(100).send('update').delay(100); + const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId); + assert(pagedResult.items.length, 4); + assert(pagedResult.items[0].text, 'foo'); + assert(pagedResult.items[1].text, 'response'); + assert(pagedResult.items[2].text, 'new response'); + assert(pagedResult.items[3].text, 'update'); }); - it(`should log delete activities`, function(done) { - var conversationId = null; - var activityId = null; - var transcriptStore = new MemoryTranscriptStore(); - var adapter = new TestAdapter(async (context) => { + it(`should log delete activities`, async function () { + let conversationId = null; + let activityId = null; + const transcriptStore = new MemoryTranscriptStore(); + const adapter = new TestAdapter(async (context) => { conversationId = context.activity.conversation.id; - if(context.activity.text === 'deleteIt') { + if (context.activity.text === 'deleteIt') { await context.deleteActivity(activityId); } else { - var activity = createReply(context.activity, 'response'); - var response = await context.sendActivity(activity); + const activity = createReply(context.activity, 'response'); + const response = await context.sendActivity(activity); activityId = response.id; } }).use(new TranscriptLoggerMiddleware(transcriptStore)); - adapter.send('foo') - .assertReply('response') - .send('deleteIt') - .delay(500).then(() => { - transcriptStore.getTranscriptActivities('test', conversationId).then(pagedResult => { - assert(pagedResult.items.length, 4); - assert(pagedResult.items[0].text, 'foo'); - assert(pagedResult.items[1].text, 'response'); - assert(pagedResult.items[2].text, 'deleteIt'); - assert(pagedResult.items[3].type, ActivityTypes.MessageDelete); - done(); - }); - }) + await adapter.send('foo').assertReply('response').send('deleteIt').delay(500); + + const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId); + + assert(pagedResult.items.length, 4); + assert(pagedResult.items[0].text, 'foo'); + assert(pagedResult.items[1].text, 'response'); + assert(pagedResult.items[2].text, 'deleteIt'); + assert(pagedResult.items[3].type, ActivityTypes.MessageDelete); + }); + + it('should filter continueConversation events', async () => { + let conversationId; + const transcriptStore = new MemoryTranscriptStore(); + const adapter = new TestAdapter(async (context) => { + conversationId = context.activity.conversation.id; + }).use(new TranscriptLoggerMiddleware(transcriptStore)); + + await adapter + .send('foo') + .send('bar') + .send({ type: ActivityTypes.Event, name: ActivityEventNames.ContinueConversation }); + + const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId); + assert.strictEqual(pagedResult.items.length, 2, 'only the two message activities should be logged'); }); it(`should not error for sent activities if no ResourceResponses are received`, async () => { @@ -154,19 +137,19 @@ describe(`TranscriptLoggerMiddleware`, function () { conversationId = context.activity.conversation.id; const typingActivity = { type: ActivityTypes.Typing, - relatesTo: context.activity.relatesTo + relatesTo: context.activity.relatesTo, }; await context.sendActivity(typingActivity); await context.sendActivity(`echo:${context.activity.text}`); - }).use(new TranscriptLoggerMiddleware(transcriptStore)); - await adapter.send('foo') - .assertReply(activity => assert.equal(activity.type, ActivityTypes.Typing)) + await adapter + .send('foo') + .assertReply((activity) => assert.equal(activity.type, ActivityTypes.Typing)) .assertReply('echo:foo') .send('bar') - .assertReply(activity => assert.equal(activity.type, ActivityTypes.Typing)) + .assertReply((activity) => assert.equal(activity.type, ActivityTypes.Typing)) .assertReply('echo:bar'); const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId); @@ -177,7 +160,7 @@ describe(`TranscriptLoggerMiddleware`, function () { assert.equal(pagedResult.items[3].text, 'bar'); assert.equal(pagedResult.items[4].type, ActivityTypes.Typing); assert.equal(pagedResult.items[5].text, 'echo:bar'); - pagedResult.items.forEach(a => { + pagedResult.items.forEach((a) => { assert(a.timestamp); }); }); @@ -203,23 +186,23 @@ describe(`TranscriptLoggerMiddleware`, function () { conversationId = context.activity.conversation.id; const typingActivity = { type: ActivityTypes.Typing, - relatesTo: context.activity.relatesTo + relatesTo: context.activity.relatesTo, }; await context.sendActivity(typingActivity); await context.sendActivity(`echo:${context.activity.text}`); - }); // Register both middleware adapter.use(new TranscriptLoggerMiddleware(transcriptStore)); adapter.use(new NoResourceResponseMiddleware()); - await adapter.send('foo') - .assertReply(activity => assert.equal(activity.type, ActivityTypes.Typing)) + await adapter + .send('foo') + .assertReply((activity) => assert.equal(activity.type, ActivityTypes.Typing)) .assertReply('echo:foo') .send('bar') - .assertReply(activity => assert.equal(activity.type, ActivityTypes.Typing)) + .assertReply((activity) => assert.equal(activity.type, ActivityTypes.Typing)) .assertReply('echo:bar'); const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId); @@ -230,7 +213,7 @@ describe(`TranscriptLoggerMiddleware`, function () { assert.equal(pagedResult.items[3].text, 'bar'); assert.equal(pagedResult.items[4].type, ActivityTypes.Typing); assert.equal(pagedResult.items[5].text, 'echo:bar'); - pagedResult.items.forEach(a => { + pagedResult.items.forEach((a) => { assert(a.timestamp); }); }); @@ -263,14 +246,13 @@ describe(`TranscriptLoggerMiddleware`, function () { adapter.use(new TranscriptLoggerMiddleware(transcriptStore)); adapter.use(new NoResourceResponseMiddleware()); - await adapter.send('foo') - .assertReply('echo:foo'); + await adapter.send('foo').assertReply('echo:foo'); const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId); assert.equal(pagedResult.items.length, 2); assert.equal(pagedResult.items[0].text, 'foo'); assert.equal(pagedResult.items[1].text, 'echo:foo'); - pagedResult.items.forEach(a => { + pagedResult.items.forEach((a) => { assert(a.id); assert(a.timestamp); }); @@ -316,81 +298,80 @@ describe(`TranscriptLoggerMiddleware`, function () { adapter.use(new TranscriptLoggerMiddleware(transcriptStore)); adapter.use(new Returns1Middleware()); - await adapter.send('foo') - .assertReply('echo:foo'); + await adapter.send('foo').assertReply('echo:foo'); const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId); assert.equal(pagedResult.items.length, 2); assert.equal(pagedResult.items[0].text, 'foo'); assert.equal(pagedResult.items[1].text, 'echo:foo'); - pagedResult.items.forEach(a => { + pagedResult.items.forEach((a) => { assert(a.id); assert(a.timestamp); }); }); - describe('\'s error handling', function () { - const originalConsoleError = console.error; - - this.afterEach(function () { - console.error = originalConsoleError; - }) - - function stubConsoleError(done, expectedErrorMessage, expectedErrors = 1) { - let errorCounter = 0; - console.error = function (error) { - // We only care about the error message on the first error. - if (errorCounter === 0) { - if (expectedErrorMessage) { - assert(error.startsWith(expectedErrorMessage), `unexpected error message received: ${ error }`); - } else { - assert(error.startsWith('TranscriptLoggerMiddleware logActivity failed'), `unexpected error message received: ${ error }`); - } - } - errorCounter++; - - if (expectedErrors === errorCounter) { - done(); - } if (errorCounter > expectedErrors) { - throw new Error(`console.error() called too many times.`); - } - } - } - - it(`should print to console errors from synchronous logActivity()`, function (done) { - stubConsoleError(done, undefined, 2); - var adapter = new TestAdapter(async (context) => { - }).use(new TranscriptLoggerMiddleware(new SyncErrorLogger())); - - adapter.send('test'); + describe("'s error handling", function () { + let sandbox; + beforeEach(() => { + sandbox = sinon.createSandbox(); }); - it(`should print to console errors from asynchronous logActivity().`, function (done) { - stubConsoleError(done, undefined, 2); - var adapter = new TestAdapter(async (context) => { - }).use(new TranscriptLoggerMiddleware(new AsyncErrorLogger())); - - adapter.send('test'); + afterEach(() => { + sandbox.restore(); }); - it(`should print to console thrown info from synchronous logActivity()`, function (done) { - stubConsoleError(done, 'TranscriptLoggerMiddleware logActivity failed: "1"'); - var adapter = new TestAdapter(async (context) => { - }).use(new TranscriptLoggerMiddleware(new SyncThrowerLogger())); - - adapter.send('test'); - }); + // Returns an adapter that is expected to throw (or, IFF `async` is true, yield a Promise rejecting) + // `expectedError`. All behavior can be verified by calling `sandbox.verify()`. + const mockedAdapter = (expectedError, async = false) => { + const logger = new ConsoleTranscriptLogger(); + + const adapter = new TestAdapter(async () => { + // noop + }).use(new TranscriptLoggerMiddleware(logger)); + + const mLogger = sandbox.mock(logger); + if (async) { + mLogger.expects('logActivity').returns(Promise.reject(expectedError)); + } else { + mLogger.expects('logActivity').throws(expectedError); + } + + const mConsole = sandbox.mock(console); + + const prefix = 'TranscriptLoggerMiddleware logActivity failed'; + if (expectedError instanceof Error) { + mConsole.expects('error').withArgs(`${prefix}: "${expectedError.message}"`).once(); + mConsole.expects('error').withArgs(expectedError.stack).once(); + } else { + mConsole + .expects('error') + .withArgs(`${prefix}: "${JSON.stringify(expectedError)}"`) + .once(); + } + + return adapter; + }; - it(`should print to console thrown info from asynchronous logActivity().`, function (done) { - stubConsoleError(done, 'TranscriptLoggerMiddleware logActivity failed: "2"'); - var adapter = new TestAdapter(async (context) => { - }).use(new TranscriptLoggerMiddleware(new AsyncThrowerLogger())); - - adapter.send('test'); + const testCases = [ + { label: 'errors', expectedError: new Error('error message') }, + { label: 'thrown info', expectedError: 10 }, + ]; + + testCases.forEach((testCase) => { + it(`should print to console ${testCase.label} from synchronous logActivity()`, async function () { + await mockedAdapter(testCase.expectedError).send('test'); + sandbox.verify(); + }); + + it(`should print to console ${testCase.label} from asynchronous logActivity().`, async function () { + await mockedAdapter(testCase.expectedError, true).send('test'); + await new Promise((resolve) => process.nextTick(resolve)); + sandbox.verify(); + }); }); }); - it(`should add resource response id to activity when activity id is empty`, function (done) { + it(`should add resource response id to activity when activity id is empty`, async function () { let conversationId = null; const transcriptStore = new MemoryTranscriptStore(); const adapter = createTestAdapterWithNoResourceResponseId(async (context) => { @@ -402,34 +383,30 @@ describe(`TranscriptLoggerMiddleware`, function () { const fooActivity = { type: ActivityTypes.Message, id: 'testFooId', - text: 'foo' + text: 'foo', }; - adapter + await adapter .send(fooActivity) // sent activities do not contain the id returned from the service, so it should be undefined here - .assertReply(activity => assert.equal(activity.id, undefined) && assert.equal(activity.text, 'echo:foo')) + .assertReply((activity) => assert.equal(activity.id, undefined) && assert.equal(activity.text, 'echo:foo')) .send('bar') - .assertReply(activity => assert.equal(activity.id, undefined) && assert.equal(activity.text, 'echo:bar')) - .then(() => { - transcriptStore.getTranscriptActivities('test', conversationId).then(pagedResult => { - assert.equal(pagedResult.items.length, 4); - assert.equal(pagedResult.items[0].text, 'foo'); - // Transcript activities should have the id present on the activity when received - assert.equal(pagedResult.items[0].id, 'testFooId'); - - assert.equal(pagedResult.items[1].text, 'echo:foo'); - assert.equal(pagedResult.items[2].text, 'bar'); - assert.equal(pagedResult.items[3].text, 'echo:bar'); - - pagedResult.items.forEach(a => { - assert(a.timestamp); - assert(a.id); - }); - done(); - }); - }) - .catch(err => done(err)); + .assertReply((activity) => assert.equal(activity.id, undefined) && assert.equal(activity.text, 'echo:bar')); + + const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId); + assert.equal(pagedResult.items.length, 4); + assert.equal(pagedResult.items[0].text, 'foo'); + // Transcript activities should have the id present on the activity when received + assert.equal(pagedResult.items[0].id, 'testFooId'); + + assert.equal(pagedResult.items[1].text, 'echo:foo'); + assert.equal(pagedResult.items[2].text, 'bar'); + assert.equal(pagedResult.items[3].text, 'echo:bar'); + + pagedResult.items.forEach((a) => { + assert(a.timestamp); + assert(a.id); + }); }); it(`should use outgoing activity's timestamp for activity id when activity id and resourceResponse is empty`, async () => { @@ -441,26 +418,27 @@ describe(`TranscriptLoggerMiddleware`, function () { timestamp = new Date(Date.now()); await context.sendActivity({ - text: `echo:${ context.activity.text }`, + text: `echo:${context.activity.text}`, timestamp: timestamp, - type: ActivityTypes.Message + type: ActivityTypes.Message, }); }).use(new TranscriptLoggerMiddleware(transcriptStore)); const fooActivity = { type: ActivityTypes.Message, id: 'testFooId', - text: 'foo' + text: 'foo', }; - await adapter.send(fooActivity) + await adapter + .send(fooActivity) // sent activities do not contain the id returned from the service, so it should be undefined here - .assertReply(activity => { + .assertReply((activity) => { assert.equal(activity.id, undefined); assert.equal(activity.text, 'echo:foo'); assert.equal(activity.timestamp, timestamp); }); - + const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId); assert.equal(pagedResult.items.length, 2); assert.equal(pagedResult.items[0].text, 'foo'); @@ -475,12 +453,12 @@ describe(`TranscriptLoggerMiddleware`, function () { // 3. The outgoing Activity.timestamp exists assert(pagedResult.items[1].id.indexOf(timestamp.getTime().toString())); assert(pagedResult.items[1].id.startsWith('g_')); - pagedResult.items.forEach(a => { + pagedResult.items.forEach((a) => { assert(a.timestamp); }); }); - it(`should use current server time for activity id when activity and resourceResponse id is empty and no activity timestamp exists`, function(done) { + it(`should use current server time for activity id when activity and resourceResponse id is empty and no activity timestamp exists`, async function () { let conversationId; const transcriptStore = new MemoryTranscriptStore(); @@ -488,44 +466,41 @@ describe(`TranscriptLoggerMiddleware`, function () { conversationId = context.activity.conversation.id; await context.sendActivity({ - text: `echo:${ context.activity.text }`, - type: ActivityTypes.Message + text: `echo:${context.activity.text}`, + type: ActivityTypes.Message, }); }).use(new TranscriptLoggerMiddleware(transcriptStore)); const fooActivity = { type: ActivityTypes.Message, id: 'testFooId', - text: 'foo' + text: 'foo', }; - adapter + await adapter .send(fooActivity) // sent activities do not contain the id returned from the service, so it should be undefined here - .assertReply(activity => { + .assertReply((activity) => { assert.equal(activity.id, undefined); assert.equal(activity.text, 'echo:foo'); - }) - .then(() => { - transcriptStore.getTranscriptActivities('test', conversationId).then(pagedResult => { - assert.equal(pagedResult.items.length, 2); - assert.equal(pagedResult.items[0].text, 'foo'); - // Transcript activities should have the id present on the activity when received - assert.equal(pagedResult.items[0].id, 'testFooId'); - - assert.equal(pagedResult.items[1].text, 'echo:foo'); - // Sent Activities in the transcript store should use the time the TranscriptLoggerMiddleware attempted - // to log the activity when the following cases are true: - // 1. The outgoing Activity.id is falsey - // 2. The ResourceResponse.id is falsey (some channels may not emit a ResourceResponse with an id value) - // 3. The outgoing Activity.timestamp is falsey - assert(pagedResult.items[1].id); - pagedResult.items.forEach(a => { - assert(a.timestamp); - }); - done(); - }); }); + + const pagedResult = await transcriptStore.getTranscriptActivities('test', conversationId); + assert.equal(pagedResult.items.length, 2); + assert.equal(pagedResult.items[0].text, 'foo'); + // Transcript activities should have the id present on the activity when received + assert.equal(pagedResult.items[0].id, 'testFooId'); + + assert.equal(pagedResult.items[1].text, 'echo:foo'); + // Sent Activities in the transcript store should use the time the TranscriptLoggerMiddleware attempted + // to log the activity when the following cases are true: + // 1. The outgoing Activity.id is falsey + // 2. The ResourceResponse.id is falsey (some channels may not emit a ResourceResponse with an id value) + // 3. The outgoing Activity.timestamp is falsey + assert(pagedResult.items[1].id); + pagedResult.items.forEach((a) => { + assert(a.timestamp); + }); }); }); @@ -537,9 +512,9 @@ function createTestAdapterWithNoResourceResponseId(logic) { .filter((a) => this.sendTraceActivities || a.type !== 'trace') .map((activity) => { this.activityBuffer.push(activity); - return { }; + return {}; }); - + return Promise.resolve(responses); } modifiedAdapter.sendActivities = sendActivities.bind(modifiedAdapter); @@ -547,7 +522,6 @@ function createTestAdapterWithNoResourceResponseId(logic) { return modifiedAdapter; } - function createReply(activity, text, locale = null) { return { type: ActivityTypes.Message, @@ -556,8 +530,12 @@ function createReply(activity, text, locale = null) { replyToId: activity.id, serviceUrl: activity.serviceUrl, channelId: activity.channelId, - conversation: { isGroup: activity.conversation.isGroup, id: activity.conversation.id, name: activity.conversation.name }, + conversation: { + isGroup: activity.conversation.isGroup, + id: activity.conversation.id, + name: activity.conversation.name, + }, text: text || '', - locale: locale || activity.locale + locale: locale || activity.locale, }; -} \ No newline at end of file +} diff --git a/libraries/botbuilder/src/botFrameworkAdapter.ts b/libraries/botbuilder/src/botFrameworkAdapter.ts index a376dc4394..f35b003d4c 100644 --- a/libraries/botbuilder/src/botFrameworkAdapter.ts +++ b/libraries/botbuilder/src/botFrameworkAdapter.ts @@ -31,6 +31,7 @@ import { TurnContext, HealthCheckResponse, HealthResults, + ActivityEventNames, } from 'botbuilder-core'; import { AuthenticationConfiguration, @@ -399,7 +400,7 @@ export class BotFrameworkAdapter const connectorClient = this.createConnectorClientInternal(reference.serviceUrl, credentials); const request: Partial = TurnContext.applyConversationReference( - { type: 'event', name: 'continueConversation' }, + { type: ActivityTypes.Event, name: ActivityEventNames.ContinueConversation }, reference, true ); @@ -493,7 +494,7 @@ export class BotFrameworkAdapter // Initialize request and copy over new conversation ID and updated serviceUrl. const request: Partial = TurnContext.applyConversationReference( - { type: 'event', name: 'createConversation' }, + { type: ActivityTypes.Event, name: ActivityEventNames.CreateConversation }, reference, true ); diff --git a/libraries/botframework-schema/src/index.ts b/libraries/botframework-schema/src/index.ts index 262a3b81ad..a8e0c528ff 100644 --- a/libraries/botframework-schema/src/index.ts +++ b/libraries/botframework-schema/src/index.ts @@ -481,7 +481,7 @@ export interface Activity { /** * The name of the operation associated with an invoke or event activity. */ - name?: string; + name?: ActivityEventNames | string; /** * A reference to another conversation or activity. */ @@ -1725,6 +1725,17 @@ export enum RoleTypes { Bot = 'bot', } +/** + * Defines values for ActivityEventNames. + * Possible values include: 'continueConversation', 'createConversation' + * @readonly + * @enum {string} + */ +export enum ActivityEventNames { + ContinueConversation = 'continueConversation', + CreateConversation = 'createConversation' +} + /** * Defines values for ActivityTypes. * Possible values include: 'message', 'contactRelationUpdate', 'conversationUpdate', 'typing',