Adds check to TranscriptLoggerMiddleware doesn't log continue conversation event activities (#4797)

* Adds check to TranscriptLoggerMiddleware doesn't log contine conversation event activities.

Changes include:
- Added check in TranscriptLoggerMiddleware to filter incoming ContinueConversation events.
- Introduced EventActivityNames class with constant strings for ContinueConversation so we don't have those strings hardcoded.
- Added also CreateConversation to the EventActivityNames helper (not related to this issue but saw it and decided to add it since it is better to have constants).
- Refactored ConversationReferenceEx to use the new constant (and use more straightforward code to create the event).
- Added tests for new logic in TranscriptLoggerMiddleware.
- Updated Skills testing projects to use the new constant

* Renamed EventActivityNames to ActivityEventNames

* Updated a couple of files that I missed after the rename.

FunctionalTests/Skills/Adaptive/AdaptiveRootBot/Middleware/LoggerMiddleware.cs

@@ -28,7 +28,7 @@ namespace Microsoft.BotBuilderSamples.AdaptiveRootBot.Middleware
         {
             // Note: skill responses will show as ContinueConversation events; we don't log those.
             // We only log incoming messages from users.
-            if (turnContext.Activity.Type != ActivityTypes.Event && turnContext.Activity.Name != "ContinueConversation")
+            if (!(turnContext.Activity.Type == ActivityTypes.Event && turnContext.Activity.Name == ActivityEventNames.ContinueConversation))
             {
                 var message = $"User said: {turnContext.Activity.Text} Type: \"{turnContext.Activity.Type}\" Name: \"{turnContext.Activity.Name}\"";
                 _logger.LogInformation(message);

FunctionalTests/Skills/DialogToDialog/DialogRootBot/Middleware/LoggerMiddleware.cs

@@ -28,7 +28,7 @@ namespace Microsoft.BotBuilderSamples.DialogRootBot.Middleware
         {
             // Note: skill responses will show as ContinueConversation events; we don't log those.
             // We only log incoming messages from users.
-            if (turnContext.Activity.Type != ActivityTypes.Event && turnContext.Activity.Name != "ContinueConversation")
+            if (!(turnContext.Activity.Type == ActivityTypes.Event && turnContext.Activity.Name == ActivityEventNames.ContinueConversation))
             {
                 var message = $"User said: {turnContext.Activity.Text} Type: \"{turnContext.Activity.Type}\" Name: \"{turnContext.Activity.Name}\"";
                 _logger.LogInformation(message);

libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs

@@ -1278,7 +1278,7 @@ namespace Microsoft.Bot.Builder
 
             // Create a conversation update activity to represent the result.
             var eventActivity = Activity.CreateEventActivity();
-            eventActivity.Name = "CreateConversation";
+            eventActivity.Name = ActivityEventNames.CreateConversation;
             eventActivity.ChannelId = channelId;
             eventActivity.ServiceUrl = serviceUrl;
             eventActivity.Id = result.ActivityId ?? Guid.NewGuid().ToString("n");

libraries/Microsoft.Bot.Builder/TranscriptLoggerMiddleware.cs

@@ -4,7 +4,6 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Bot.Schema;
@@ -17,7 +16,7 @@ namespace Microsoft.Bot.Builder
     /// </summary>
     public class TranscriptLoggerMiddleware : IMiddleware
     {
-        private static readonly JsonSerializerSettings _jsonSettings = new JsonSerializerSettings() { NullValueHandling = NullValueHandling.Ignore };
+        private static readonly JsonSerializerSettings _jsonSettings = new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore };
         private readonly ITranscriptLogger _logger;
 
         /// <summary>
@@ -41,22 +40,23 @@ namespace Microsoft.Bot.Builder
         /// <seealso cref="Bot.Schema.IActivity"/>
         public async Task OnTurnAsync(ITurnContext turnContext, NextDelegate nextTurn, CancellationToken cancellationToken)
         {
-            Queue<IActivity> transcript = new Queue<IActivity>();
+            var transcript = new Queue<IActivity>();
 
             // log incoming activity at beginning of turn
             if (turnContext.Activity != null)
             {
-                if (turnContext.Activity.From == null)
+                turnContext.Activity.From ??= new ChannelAccount();
-                {
-                    turnContext.Activity.From = new ChannelAccount();
-                }
 
                 if (string.IsNullOrEmpty((string)turnContext.Activity.From.Properties["role"]))
                 {
                     turnContext.Activity.From.Properties["role"] = "user";
                 }
 
-                LogActivity(transcript, CloneActivity(turnContext.Activity));
+                // We should not log ContinueConversation events used by skills to initialize the middleware.
+                if (!(turnContext.Activity.Type == ActivityTypes.Event && turnContext.Activity.Name == ActivityEventNames.ContinueConversation))
+                {
+                    LogActivity(transcript, CloneActivity(turnContext.Activity));
+                }
             }
 
             // hook up onSend pipeline
@@ -95,12 +95,12 @@ namespace Microsoft.Bot.Builder
                 // add MessageDelete activity
                 // log as MessageDelete activity
                 var deleteActivity = new Activity
-                {
+                    {
-                    Type = ActivityTypes.MessageDelete,
+                        Type = ActivityTypes.MessageDelete,
-                    Id = reference.ActivityId,
+                        Id = reference.ActivityId,
-                }
+                    }
-                .ApplyConversationReference(reference, isIncoming: false)
+                    .ApplyConversationReference(reference, isIncoming: false)
-                .AsMessageDeleteActivity();
+                    .AsMessageDeleteActivity();
 
                 LogActivity(transcript, deleteActivity);
             });
@@ -163,11 +163,7 @@ namespace Microsoft.Bot.Builder
 
         private static void LogActivity(Queue<IActivity> transcript, IActivity activity)
         {
-            if (activity.Timestamp == null)
+            activity.Timestamp ??= DateTime.UtcNow;
-            {
-                activity.Timestamp = DateTime.UtcNow;
-            }
-
             transcript.Enqueue(activity);
         }
     }

libraries/Microsoft.Bot.Connector/Conversations.cs

@@ -275,7 +275,7 @@ namespace Microsoft.Bot.Connector
                 Dictionary<string, object> tracingParameters = new Dictionary<string, object>();
                 tracingParameters.Add("parameters", parameters);
                 tracingParameters.Add("cancellationToken", cancellationToken);
-                ServiceClientTracing.Enter(_invocationId, this, "CreateConversation", tracingParameters);
+                ServiceClientTracing.Enter(_invocationId, this, ActivityEventNames.CreateConversation, tracingParameters);
             }
             // Construct URL
             var _baseUrl = Client.BaseUri.AbsoluteUri;

libraries/Microsoft.Bot.Schema/ActivityEventNames.cs

@@ -0,0 +1,21 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+namespace Microsoft.Bot.Schema
+{
+    /// <summary>
+    /// Define values for common event names used by activities of type <see cref="ActivityTypes.Event"/>.
+    /// </summary>
+    public static class ActivityEventNames
+    {
+        /// <summary>
+        /// The event name for continuing a conversation.
+        /// </summary>
+        public const string ContinueConversation = "ContinueConversation";
+
+        /// <summary>
+        /// The event name for creating a conversation.
+        /// </summary>
+        public const string CreateConversation = "CreateConversation";
+    }
+}

libraries/Microsoft.Bot.Schema/ConversationReferenceEx.cs

@@ -16,17 +16,18 @@ namespace Microsoft.Bot.Schema
         /// <returns>Continuation activity.</returns>
         public Activity GetContinuationActivity()
         {
-            var activity = Activity.CreateEventActivity();
+            return new Activity(ActivityTypes.Event)
-            activity.Name = "ContinueConversation";
+            {
-            activity.Id = Guid.NewGuid().ToString();
+                Name = ActivityEventNames.ContinueConversation,
-            activity.ChannelId = this.ChannelId;
+                Id = Guid.NewGuid().ToString(),
-            (activity as Activity).Locale = this.Locale;
+                ChannelId = ChannelId,
-            activity.ServiceUrl = this.ServiceUrl;
+                Locale = Locale,
-            activity.Conversation = this.Conversation;
+                ServiceUrl = ServiceUrl,
-            activity.Recipient = this.Bot;
+                Conversation = Conversation,
-            activity.From = this.User;
+                Recipient = Bot,
-            activity.RelatesTo = this;
+                From = User,
-            return (Activity)activity;
+                RelatesTo = this
+            };
         }
     }
 }

tests/Microsoft.Bot.Builder.Azure.Tests/AzureQueueTests.cs

@@ -59,7 +59,7 @@ namespace Microsoft.Bot.Builder.Azure.Tests
                 var messageJson = Encoding.UTF8.GetString(Convert.FromBase64String(message.MessageText));
                 var activity = JsonConvert.DeserializeObject<Activity>(messageJson);
                 Assert.Equal(ActivityTypes.Event, activity.Type);
-                Assert.Equal("ContinueConversation", activity.Name);
+                Assert.Equal(ActivityEventNames.ContinueConversation, activity.Name);
                 Assert.Equal("foo", activity.Value);
                 Assert.NotNull(activity.RelatesTo);
                 var cr2 = activity.GetConversationReference();

tests/Microsoft.Bot.Builder.Tests/BotFrameworkAdapterTests.cs

@@ -64,7 +64,7 @@ namespace Microsoft.Bot.Builder.Tests
             const string conversationIdName = "Id";
             const string conversationIdValue = "NewConversationId";
             const string tenantIdValue = "theTenantId";
-            const string eventActivityName = "CreateConversation";
+            const string eventActivityName = ActivityEventNames.CreateConversation;
 
             Task<HttpResponseMessage> CreateResponseMessage()
             {

tests/Microsoft.Bot.Builder.Tests/TranscriptLoggerMiddlewareTests.cs

@@ -0,0 +1,46 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Threading.Tasks;
+using Microsoft.Bot.Builder.Adapters;
+using Microsoft.Bot.Schema;
+using Xunit;
+
+namespace Microsoft.Bot.Builder.Tests
+{
+    public class TranscriptLoggerMiddlewareTests
+    {
+        [Fact]
+        public async Task ShouldNotLogContinueConversation()
+        {
+            var transcriptStore = new MemoryTranscriptStore();
+            var sut = new TranscriptLoggerMiddleware(transcriptStore);
+
+            var conversationId = Guid.NewGuid().ToString();
+            var adapter = new TestAdapter(TestAdapter.CreateConversation(conversationId))
+                .Use(sut);
+
+            await new TestFlow(adapter, async (context, cancellationToken) =>
+                {
+                    await context.SendActivityAsync("bar", cancellationToken: cancellationToken);
+                })
+                .Send("foo")
+                .AssertReply(async activity =>
+                {
+                    Assert.Equal("bar", ((Activity)activity).Text);
+                    var activities = await transcriptStore.GetTranscriptActivitiesAsync(activity.ChannelId, conversationId);
+                    Assert.Equal(2, activities.Items.Length);
+                })
+                .Send(new Activity(ActivityTypes.Event) { Name = ActivityEventNames.ContinueConversation })
+                .AssertReply(async activity =>
+                {
+                    // Ensure the event hasn't been added to the transcript.
+                    var activities = await transcriptStore.GetTranscriptActivitiesAsync(activity.ChannelId, conversationId);
+                    Assert.DoesNotContain(activities.Items, a => ((Activity)a).Type == ActivityTypes.Event && ((Activity)a).Name == ActivityEventNames.ContinueConversation);
+                    Assert.Equal(3, activities.Items.Length);
+                })
+                .StartTestAsync();
+        }
+    }
+}