Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public async Task OnTurnAsync(ITurnContext turnContext, NextDelegate next, Cance
{
// 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public async Task OnTurnAsync(ITurnContext turnContext, NextDelegate next, Cance
{
// 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);
Expand Down
2 changes: 1 addition & 1 deletion libraries/Microsoft.Bot.Builder/BotFrameworkAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1278,7 +1278,7 @@ public virtual async Task CreateConversationAsync(string channelId, string servi

// 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");
Expand Down
34 changes: 15 additions & 19 deletions libraries/Microsoft.Bot.Builder/TranscriptLoggerMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>
Expand All @@ -41,22 +40,23 @@ public TranscriptLoggerMiddleware(ITranscriptLogger transcriptLogger)
/// <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
Expand Down Expand Up @@ -95,12 +95,12 @@ public async Task OnTurnAsync(ITurnContext turnContext, NextDelegate nextTurn, C
// add MessageDelete activity
// log as MessageDelete activity
var deleteActivity = new Activity
{
Type = ActivityTypes.MessageDelete,
Id = reference.ActivityId,
}
.ApplyConversationReference(reference, isIncoming: false)
.AsMessageDeleteActivity();
{
Type = ActivityTypes.MessageDelete,
Id = reference.ActivityId,
}
.ApplyConversationReference(reference, isIncoming: false)
.AsMessageDeleteActivity();

LogActivity(transcript, deleteActivity);
});
Expand Down Expand Up @@ -163,11 +163,7 @@ private static IActivity EnsureActivityHasId(IActivity activity)

private static void LogActivity(Queue<IActivity> transcript, IActivity activity)
{
if (activity.Timestamp == null)
{
activity.Timestamp = DateTime.UtcNow;
}

activity.Timestamp ??= DateTime.UtcNow;
transcript.Enqueue(activity);
}
}
Expand Down
2 changes: 1 addition & 1 deletion libraries/Microsoft.Bot.Connector/Conversations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ public Conversations(ConnectorClient client)
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;
Expand Down
21 changes: 21 additions & 0 deletions libraries/Microsoft.Bot.Schema/ActivityEventNames.cs
Original file line number Diff line number Diff line change
@@ -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";
}
}
23 changes: 12 additions & 11 deletions libraries/Microsoft.Bot.Schema/ConversationReferenceEx.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,18 @@ public partial class ConversationReference
/// <returns>Continuation activity.</returns>
public Activity GetContinuationActivity()
{
var activity = Activity.CreateEventActivity();
activity.Name = "ContinueConversation";
activity.Id = Guid.NewGuid().ToString();
activity.ChannelId = this.ChannelId;
(activity as Activity).Locale = this.Locale;
activity.ServiceUrl = this.ServiceUrl;
activity.Conversation = this.Conversation;
activity.Recipient = this.Bot;
activity.From = this.User;
activity.RelatesTo = this;
return (Activity)activity;
return new Activity(ActivityTypes.Event)
{
Name = ActivityEventNames.ContinueConversation,
Id = Guid.NewGuid().ToString(),
ChannelId = ChannelId,
Locale = Locale,
ServiceUrl = ServiceUrl,
Conversation = Conversation,
Recipient = Bot,
From = User,
RelatesTo = this
};
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public async Task ContinueConversationLaterTests()
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public async Task CreateConversationOverloadProperlySetsTenantId()
const string conversationIdName = "Id";
const string conversationIdValue = "NewConversationId";
const string tenantIdValue = "theTenantId";
const string eventActivityName = "CreateConversation";
const string eventActivityName = ActivityEventNames.CreateConversation;

Task<HttpResponseMessage> CreateResponseMessage()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
}