From 572851baa181a1680e810380033fcaca12d0078c Mon Sep 17 00:00:00 2001 From: Gary Pretty Date: Wed, 25 Mar 2020 15:34:26 +0000 Subject: [PATCH] Moved text normalization logic earlier and amended markdown handling (#212) * Moved logic for text normalization earlier in the pipeline into the MergeActivities method. * Modified markdown renderer to use period instead of new line. Amended list rendering. Added tests. Changed default to markdown. --- .../AlexaRequestMapper.cs | 26 +++++----- .../AlexaMarkdownToPlaintextRenderer.cs | 18 +++---- .../AlexaRequestMapperTests.cs | 52 +++++++++++++++++++ 3 files changed, 75 insertions(+), 21 deletions(-) diff --git a/libraries/Bot.Builder.Community.Adapters.Alexa.Core/AlexaRequestMapper.cs b/libraries/Bot.Builder.Community.Adapters.Alexa.Core/AlexaRequestMapper.cs index 6c92579d..8ceb7277 100644 --- a/libraries/Bot.Builder.Community.Adapters.Alexa.Core/AlexaRequestMapper.cs +++ b/libraries/Bot.Builder.Community.Adapters.Alexa.Core/AlexaRequestMapper.cs @@ -83,7 +83,7 @@ public SkillResponse ActivityToResponse(Activity activity, SkillRequest alexaReq Response = new ResponseBody() }; - if (activity == null || alexaRequest.Request is SessionEndedRequest) + if (activity == null || activity.Type != ActivityTypes.Message || alexaRequest.Request is SessionEndedRequest) { response.Response.ShouldEndSession = true; response.Response.OutputSpeech = new PlainTextOutputSpeech @@ -99,7 +99,7 @@ public SkillResponse ActivityToResponse(Activity activity, SkillRequest alexaReq } else { - response.Response.OutputSpeech = new PlainTextOutputSpeech(NormalizeActivityText(activity.TextFormat, activity.Text)); + response.Response.OutputSpeech = new PlainTextOutputSpeech(activity.Text); } ProcessActivityAttachments(activity, response); @@ -113,7 +113,7 @@ public SkillResponse ActivityToResponse(Activity activity, SkillRequest alexaReq break; case InputHints.ExpectingInput: response.Response.ShouldEndSession = false; - response.Response.Reprompt = new Reprompt(NormalizeActivityText(activity.TextFormat, activity.Text)); + response.Response.Reprompt = new Reprompt(activity.Text); break; default: response.Response.ShouldEndSession = _options.ShouldEndSessionByDefault; @@ -136,25 +136,27 @@ public SkillResponse ActivityToResponse(Activity activity, SkillRequest alexaReq /// Activity public Activity MergeActivities(IList activities) { - if (activities == null || activities.Count == 0) + if (activities == null || activities.All(a => a.Type != ActivityTypes.Message)) { return null; } - var activity = activities.Last(); + var messageActivities = activities.Where(a => a.Type == ActivityTypes.Message).ToList(); - if (activities.Any(a => !string.IsNullOrEmpty(a.Speak))) + var activity = messageActivities.Last(); + + if (messageActivities.Any(a => !string.IsNullOrEmpty(a.Speak))) { - var speakText = string.Join("", activities - .Select(a => !string.IsNullOrEmpty(a.Speak) ? StripSpeakTag(a.Speak) : a.Text) + var speakText = string.Join("", messageActivities + .Select(a => !string.IsNullOrEmpty(a.Speak) ? StripSpeakTag(a.Speak) : NormalizeActivityText(a.TextFormat, a.Text)) .Where(s => !string.IsNullOrEmpty(s)) .Select(s => s)); activity.Speak = $"{speakText}"; } - activity.Text = string.Join(". ", activities - .Select(a => a.Text) + activity.Text = string.Join(". ", messageActivities + .Select(a => NormalizeActivityText(a.TextFormat, a.Text)) .Where(s => !string.IsNullOrEmpty(s)) .Select(s => s.Trim(new char[] { ' ', '.' }))); @@ -284,10 +286,10 @@ private string NormalizeActivityText(string textFormat, string text) return string.Empty; } - // Default to plain text if it isn't specified. + // Default to markdown if it isn't specified. if (textFormat == null) { - textFormat = TextFormatTypes.Plain; + textFormat = TextFormatTypes.Markdown; } string plainText; diff --git a/libraries/Bot.Builder.Community.Adapters.Alexa.Core/Utility/AlexaMarkdownToPlaintextRenderer.cs b/libraries/Bot.Builder.Community.Adapters.Alexa.Core/Utility/AlexaMarkdownToPlaintextRenderer.cs index af280c94..a27367a8 100644 --- a/libraries/Bot.Builder.Community.Adapters.Alexa.Core/Utility/AlexaMarkdownToPlaintextRenderer.cs +++ b/libraries/Bot.Builder.Community.Adapters.Alexa.Core/Utility/AlexaMarkdownToPlaintextRenderer.cs @@ -16,14 +16,14 @@ private class RemoveMarkupRenderer : MarkdownRenderer { private const string ListItemMarker = "$$ListItemMarker$$"; - public override string Blockquote(string quote) => string.Concat(Environment.NewLine, quote, Environment.NewLine); - public override string Br() => Environment.NewLine; + public override string Blockquote(string quote) => string.Concat(quote, ". "); + public override string Br() => ". "; public override string Code(string code, string lang, bool escaped) => code; public override string Codespan(string text) => text; public override string Del(string text) => text; public override string Em(string text) => text; - public override string Heading(string text, int level, string raw) => string.Concat(Environment.NewLine, text, Environment.NewLine); - public override string Hr() => Environment.NewLine; + public override string Heading(string text, int level, string raw) => string.Concat(text, ". "); + public override string Hr() => ". "; public override string Html(string html) => string.Empty; public override string Image(string href, string title, string text) => title ?? text; public override string Link(string href, string title, string text) => $"{title ?? text} {href}"; @@ -33,17 +33,17 @@ public override string List(string body, bool ordered, int start) { for (int marker = start, markerIndex = body.IndexOf(ListItemMarker); markerIndex >= 0; markerIndex = body.IndexOf(ListItemMarker), ++marker) { - body = body.Substring(0, markerIndex) + Environment.NewLine + marker + " " + body.Substring(markerIndex + ListItemMarker.Length); + body = body.Substring(0, markerIndex) + marker + ". " + body.Substring(markerIndex + ListItemMarker.Length); } } else { - body = body.Replace(ListItemMarker, Environment.NewLine); + body = body.Replace(ListItemMarker, string.Empty); } - return body; + return $"{body.Trim().TrimEnd(',')}. "; } - public override string ListItem(string text) => $"{ListItemMarker}{text}"; - public override string Paragraph(string text) => string.Concat(Environment.NewLine, text, Environment.NewLine); + public override string ListItem(string text) => $"{ListItemMarker}{text}, "; + public override string Paragraph(string text) => string.Concat(text.TrimEnd('.'), ". "); public override string Strong(string text) => text; public override string Table(string header, string body) => string.Empty; public override string TableCell(string content, TableCellFlags flags) => string.Empty; diff --git a/tests/Bot.Builder.Community.Adapters.Alexa.Tests/AlexaRequestMapperTests.cs b/tests/Bot.Builder.Community.Adapters.Alexa.Tests/AlexaRequestMapperTests.cs index 90abdf0c..0bb6c113 100644 --- a/tests/Bot.Builder.Community.Adapters.Alexa.Tests/AlexaRequestMapperTests.cs +++ b/tests/Bot.Builder.Community.Adapters.Alexa.Tests/AlexaRequestMapperTests.cs @@ -8,6 +8,7 @@ using Microsoft.Extensions.Logging; using Moq; using System.Collections.Generic; +using System.Text; using Alexa.NET.Response.Directive; using Alexa.NET.Response.Directive.Templates; using Alexa.NET.Response.Directive.Templates.Types; @@ -96,6 +97,21 @@ public void MergeActivitiesReturnsCorrectlyJoinedText() Assert.Equal("This is the first activity. This is the second activity", processActivityResult.Text); } + [Fact] + public void MergeActivitiesIgnoresNonMessageActivities() + { + var alexaAdapter = new AlexaRequestMapper(); + + var firstActivity = MessageFactory.Text("This is the first activity."); + var traceActivity = Activity.CreateTraceActivity("This is a trace") as Activity; + var secondActivity = MessageFactory.Text("This is the second activity"); + var typingActivity = Activity.CreateTypingActivity() as Activity; + + var processActivityResult = alexaAdapter.MergeActivities(new List() { firstActivity, traceActivity, secondActivity, typingActivity }); + + Assert.Equal("This is the first activity. This is the second activity", processActivityResult.Text); + } + [Fact] public void MergeActivitiesReturnsCorrectlyJoinedSpeakWithSsml() { @@ -113,6 +129,42 @@ public void MergeActivitiesReturnsCorrectlyJoinedSpeakWithSsml() processActivityResult.Speak); } + [Fact] + public void MergeActivitiesCorrectlyConvertsMarkdownToPlainText() + { + var alexaAdapter = new AlexaRequestMapper(); + + // Note: The input activities deliberately have an activity where the speak tag + // is included and one activity where it is not, to ensure the stripping / wrapping + // of the speak tag is handled correctly. + + var message = new StringBuilder(); + message.AppendLine("**This** ~~is~~ ***the*** first activity."); + message.AppendLine("# Heading 1"); + message.AppendLine("This is another paragraph."); + message.AppendLine("- Item 1"); + message.AppendLine("- Item 2"); + message.AppendLine("- Item 3"); + message.AppendLine(""); + message.AppendLine("## Heading 2"); + message.AppendLine("1. Item 1"); + message.AppendLine("2. Item 2"); + message.AppendLine("3. Item 3"); + message.AppendLine(""); + message.AppendLine("More info [visit our web site](www.microsoft.com)"); + + var firstActivity = MessageFactory.Text(message.ToString(), "This isthe first activity SSML"); + var secondActivity = MessageFactory.Text("This is the second activity.", "This is the second activity SSML"); + + var processActivityResult = alexaAdapter.MergeActivities(new List() { firstActivity, secondActivity }); + + Assert.Equal("This is the first activity. Heading 1. This is another paragraph. Item 1, Item 2, Item 3. Heading 2. 1. Item 1, 2. Item 2, 3. Item 3. More info visit our web site www.microsoft.com. This is the second activity", + processActivityResult.Text); + + Assert.Equal("This isthe first activity SSMLThis is the second activity SSML", + processActivityResult.Speak); + } + [Fact] public void PlainTextMessageActivityConverted() {