Skip to content

Cherry-Pick to 4.12: Change ChoiceSet to support LG (#5309) #5312

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 12, 2021
Merged

Cherry-Pick to 4.12: Change ChoiceSet to support LG (#5309) #5312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d5b068
Change ChoiceSet to support LG (#5309)
Mar 12, 2021
22654aa
Fix warnings as errors (#5274)
EricDahlvang Mar 4, 2021
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
2 changes: 1 addition & 1 deletion libraries/Adapters/Microsoft.Bot.Builder.Adapters.Twilio/TwilioClientWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public virtual bool ValidateSignature(HttpRequest httpRequest, Dictionary<string

var twilioSignature = httpRequest.Headers.ContainsKey(TwilioSignature)
? httpRequest.Headers[TwilioSignature].ToString()
: throw new ArgumentNullException($"HttpRequest is missing \"{TwilioSignature}\"");
: throw new ArgumentException($"HttpRequest is missing \"{TwilioSignature}\"");

if (string.IsNullOrWhiteSpace(urlString))
{
Expand Down
11 changes: 10 additions & 1 deletion libraries/AdaptiveExpressions/ExpressionProperties/ExpressionProperty.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,16 @@ public virtual (T Value, string Error) TryGetValue(object data)
{
if (_expression == null && ExpressionText != null)
{
_expression = Expression.Parse(this.ExpressionText.TrimStart('='));
try
{
_expression = Expression.Parse(this.ExpressionText.TrimStart('='));
}
#pragma warning disable CA1031 // Do not catch general exception types
catch (Exception err)
#pragma warning restore CA1031 // Do not catch general exception types
{
return (default(T), err.Message);
}
}

if (_expression != null)
Expand Down
12 changes: 8 additions & 4 deletions libraries/Microsoft.Bot.Builder.AI.Orchestrator/OrchestratorAdaptiveRecognizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@ public OrchestratorAdaptiveRecognizer(string modelFolder, string snapshotFile, I
_resolver = resolver;
if (modelFolder == null)
{
throw new ArgumentNullException($"Missing `ModelFolder` information.");
throw new ArgumentNullException(nameof(modelFolder));
}

if (snapshotFile == null)
{
throw new ArgumentNullException($"Missing `SnapshotFile` information.");
throw new ArgumentNullException(nameof(snapshotFile));
}

_modelFolder = modelFolder;
Expand Down Expand Up @@ -226,12 +226,16 @@ private void InitializeModel()
{
if (_modelFolder == null)
{
throw new ArgumentNullException($"Missing `ModelFolder` information.");
#pragma warning disable CA2208 // Instantiate argument exceptions correctly
Copy link
Contributor

@gabog gabog Mar 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is a cherry pick but I think it may have been better to use InvalidOperationException here with a descriptive message rather than argumentnullexception with a pragma (The exception that is thrown when a method call is invalid for the object's current state.).

throw new ArgumentNullException("ModelFolder");
#pragma warning restore CA2208 // Instantiate argument exceptions correctly
}

if (_snapshotFile == null)
{
throw new ArgumentNullException($"Missing `SnapshotFile` information.");
#pragma warning disable CA2208 // Instantiate argument exceptions correctly
throw new ArgumentNullException("SnapshotFile");
#pragma warning restore CA2208 // Instantiate argument exceptions correctly
}

if (_resolver != null)
Expand Down
10 changes: 5 additions & 5 deletions libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsTranscriptStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,12 @@ public async Task<PagedResult<IActivity>> GetTranscriptActivitiesAsync(string ch

if (string.IsNullOrEmpty(channelId))
{
throw new ArgumentNullException($"missing {nameof(channelId)}");
throw new ArgumentNullException(nameof(channelId));
}

if (string.IsNullOrEmpty(conversationId))
{
throw new ArgumentNullException($"missing {nameof(conversationId)}");
throw new ArgumentNullException(nameof(conversationId));
}

var pagedResult = new PagedResult<IActivity>();
Expand Down Expand Up @@ -231,7 +231,7 @@ public async Task<PagedResult<TranscriptInfo>> ListTranscriptsAsync(string chann

if (string.IsNullOrEmpty(channelId))
{
throw new ArgumentNullException($"missing {nameof(channelId)}");
throw new ArgumentNullException(nameof(channelId));
}

string token = null;
Expand Down Expand Up @@ -291,12 +291,12 @@ public async Task DeleteTranscriptAsync(string channelId, string conversationId)
{
if (string.IsNullOrEmpty(channelId))
{
throw new ArgumentNullException($"{nameof(channelId)} should not be null");
throw new ArgumentNullException(nameof(channelId));
}

if (string.IsNullOrEmpty(conversationId))
{
throw new ArgumentNullException($"{nameof(conversationId)} should not be null");
throw new ArgumentNullException(nameof(conversationId));
}

string token = null;
Expand Down
10 changes: 5 additions & 5 deletions libraries/Microsoft.Bot.Builder.Azure/AzureBlobTranscriptStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ public async Task LogActivityAsync(IActivity activity)
{
if (string.IsNullOrEmpty(channelId))
{
throw new ArgumentNullException($"missing {nameof(channelId)}");
throw new ArgumentNullException(nameof(channelId));
}

if (string.IsNullOrEmpty(conversationId))
{
throw new ArgumentNullException($"missing {nameof(conversationId)}");
throw new ArgumentNullException(nameof(conversationId));
}

var pagedResult = new PagedResult<IActivity>();
Expand Down Expand Up @@ -227,7 +227,7 @@ public async Task<PagedResult<TranscriptInfo>> ListTranscriptsAsync(string chann
{
if (string.IsNullOrEmpty(channelId))
{
throw new ArgumentNullException($"missing {nameof(channelId)}");
throw new ArgumentNullException(nameof(channelId));
}

var dirName = GetDirName(channelId);
Expand Down Expand Up @@ -291,12 +291,12 @@ public async Task DeleteTranscriptAsync(string channelId, string conversationId)
{
if (string.IsNullOrEmpty(channelId))
{
throw new ArgumentNullException($"{nameof(channelId)} should not be null");
throw new ArgumentNullException(nameof(channelId));
}

if (string.IsNullOrEmpty(conversationId))
{
throw new ArgumentNullException($"{nameof(conversationId)} should not be null");
throw new ArgumentNullException(nameof(conversationId));
}

var dirName = GetDirName(channelId, conversationId);
Expand Down
4 changes: 3 additions & 1 deletion libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Actions/ContinueConversationLater.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ public ContinueConversationLater([CallerFilePath] string callerPath = "", [Calle
date = date.ToUniversalTime();
if (date <= DateTime.UtcNow)
{
throw new ArgumentOutOfRangeException($"{nameof(Date)} must be in the future");
#pragma warning disable CA2208 // Instantiate argument exceptions correctly
throw new ArgumentOutOfRangeException(nameof(Date), $"{nameof(Date)} must be in the future");
#pragma warning restore CA2208 // Instantiate argument exceptions correctly
}

// create ContinuationActivity from the conversation reference.
Expand Down
27 changes: 15 additions & 12 deletions libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Input/ChoiceInput.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,7 @@ protected override object OnInitializeOptions(DialogContext dc, object options)
op = new ChoiceInputOptions();
}

var (choices, error) = Choices.TryGetValue(dc.State);
if (error != null)
{
throw new InvalidOperationException(error);
}

op.Choices = choices;
op.Choices = GetChoiceSetAsync(dc).GetAwaiter().GetResult();
}

return base.OnInitializeOptions(dc, op);
Expand Down Expand Up @@ -219,14 +213,23 @@ protected override async Task<IActivity> OnRenderPromptAsync(DialogContext dc, I
var prompt = await base.OnRenderPromptAsync(dc, state, cancellationToken).ConfigureAwait(false);
var channelId = dc.Context.Activity.ChannelId;
var choiceOptions = ChoiceOptions?.GetValue(dc.State) ?? DefaultChoiceOptions[locale];
var options = dc.State.GetValue<ChoiceInputOptions>(ThisPath.Options);

return AppendChoices(prompt.AsMessageActivity(), channelId, options.Choices, Style.GetValue(dc.State), choiceOptions);
}

var (choices, error) = Choices.TryGetValue(dc.State);
if (error != null)
private async Task<ChoiceSet> GetChoiceSetAsync(DialogContext dc)
{
if (Choices.ExpressionText != null && Choices.ExpressionText.TrimStart().StartsWith("${", StringComparison.InvariantCultureIgnoreCase))
{
throw new InvalidOperationException(error);
// use ITemplate<ChocieSet> to bind (aka LG)
return await new ChoiceSet(Choices.ExpressionText).BindAsync(dc, dc.State).ConfigureAwait(false);
}
else
{
// use Expression to bind
return Choices.TryGetValue(dc.State).Value;
}

return AppendChoices(prompt.AsMessageActivity(), channelId, choices, Style.GetValue(dc.State), choiceOptions);
}

private string DetermineCulture(DialogContext dc, FindChoicesOptions opt = null)
Expand Down
60 changes: 53 additions & 7 deletions libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Input/ChoiceSet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Bot.Builder.Dialogs.Choices;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Newtonsoft.Json.Serialization;

namespace Microsoft.Bot.Builder.Dialogs.Adaptive.Input
{
/// <summary>
/// Defines ChoiceSet collection.
/// </summary>
[JsonConverter(typeof(ChoiceSetConverter))]
public class ChoiceSet : List<Choice>
public class ChoiceSet : List<Choice>, ITemplate<ChoiceSet>
{
private string template;

/// <summary>
/// Initializes a new instance of the <see cref="ChoiceSet"/> class.
/// </summary>
Expand All @@ -33,18 +40,21 @@ public ChoiceSet(IEnumerable<Choice> choices)
/// <param name="obj">Choice values.</param>
public ChoiceSet(object obj)
{
// support string[] => choice[]
if (obj is IEnumerable<string> strings)
if (obj is string template)
{
this.template = template;
}
else if (obj is IEnumerable<string> strings)
{
// support string[] => choice[]
foreach (var str in strings)
{
this.Add(new Choice(str));
}
}

// support JArray to => choice
if (obj is JArray array)
else if (obj is JArray array)
{
// support JArray to => choice
if (array.HasValues)
{
foreach (var element in array)
Expand Down Expand Up @@ -79,5 +89,41 @@ public ChoiceSet(object obj)
/// </summary>
/// <param name="value"><see cref="JToken"/> expression.</param>
public static implicit operator ChoiceSet(JToken value) => new ChoiceSet(value);

/// <inheritdoc/>
public async Task<ChoiceSet> BindAsync(DialogContext dialogContext, object data = null, CancellationToken cancellationToken = default)
{
if (this.template == null)
{
return this;
}

var languageGenerator = dialogContext.Services.Get<LanguageGenerator>() ?? throw new MissingMemberException(nameof(LanguageGeneration));
var lgResult = await languageGenerator.GenerateAsync(dialogContext, this.template, dialogContext.State).ConfigureAwait(false);
if (lgResult is ChoiceSet cs)
{
return cs;
}
else if (lgResult is string str)
{
try
{
var jObj = (JToken)JsonConvert.DeserializeObject(str);

if (jObj is JArray jarr)
{
return new ChoiceSet(jarr);
}

return jObj.ToObject<ChoiceSet>();
}
catch (JsonReaderException)
{
return new ChoiceSet(str.Split('|').Select(t => t.Trim()));
}
}

return new ChoiceSet(lgResult);
}
}
}
39 changes: 32 additions & 7 deletions libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Input/ConfirmInput.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public ConfirmInput([CallerFilePath] string callerPath = "", [CallerLineNumber]
/// <param name="dc">The <see cref="DialogContext"/> for the current turn of conversation.</param>
/// <param name="cancellationToken">Optional, the <see cref="CancellationToken"/> that can be used by other objects or threads to receive notice of cancellation.</param>
/// <returns>InputState which reflects whether input was recognized as valid or not.</returns>
protected override Task<InputState> OnRecognizeInputAsync(DialogContext dc, CancellationToken cancellationToken = default)
protected async override Task<InputState> OnRecognizeInputAsync(DialogContext dc, CancellationToken cancellationToken = default)
{
var input = dc.State.GetValue<object>(VALUE_PROPERTY);
if (dc.Context.Activity.Type == ActivityTypes.Message)
Expand All @@ -120,11 +120,11 @@ protected override Task<InputState> OnRecognizeInputAsync(DialogContext dc, Canc
}
}

return Task.FromResult(InputState.Valid);
return InputState.Valid;
}
else
{
return Task.FromResult(InputState.Unrecognized);
return InputState.Unrecognized;
}
}
else
Expand All @@ -137,7 +137,8 @@ protected override Task<InputState> OnRecognizeInputAsync(DialogContext dc, Canc
if (!choiceOptions.IncludeNumbers.HasValue || choiceOptions.IncludeNumbers.Value)
{
// The text may be a number in which case we will interpret that as a choice.
var confirmChoices = ConfirmChoices?.GetValue(dc.State) ?? new List<Choice>() { defaults.Item1, defaults.Item2 };
var confirmChoices = await GetConfirmChoicesAsync(dc, defaults).ConfigureAwait(false);

var secondAttemptResults = ChoiceRecognizers.RecognizeChoices(input.ToString(), confirmChoices);
if (secondAttemptResults.Count > 0)
{
Expand All @@ -146,13 +147,13 @@ protected override Task<InputState> OnRecognizeInputAsync(DialogContext dc, Canc
}
else
{
return Task.FromResult(InputState.Unrecognized);
return InputState.Unrecognized;
}
}
}
}

return Task.FromResult(InputState.Valid);
return InputState.Valid;
}

/// <summary>
Expand All @@ -169,7 +170,7 @@ protected override async Task<IActivity> OnRenderPromptAsync(DialogContext dc, I
var culture = DetermineCulture(dc);
var defaults = DefaultChoiceOptions[culture];
var choiceOptions = ChoiceOptions?.GetValue(dc.State) ?? defaults.Item3;
var confirmChoices = ConfirmChoices?.GetValue(dc.State) ?? new List<Choice>() { defaults.Item1, defaults.Item2 };
var confirmChoices = await GetConfirmChoicesAsync(dc, defaults).ConfigureAwait(false);

var prompt = await base.OnRenderPromptAsync(dc, state, cancellationToken).ConfigureAwait(false);
var (style, _) = Style.TryGetValue(dc.State);
Expand All @@ -189,5 +190,29 @@ private string DetermineCulture(DialogContext dc)

return culture;
}

private async Task<ChoiceSet> GetConfirmChoicesAsync(DialogContext dc, (Choice, Choice, ChoiceFactoryOptions) defaults)
{
ChoiceSet confirmChoices = null;
if (ConfirmChoices != null)
{
if (ConfirmChoices.ExpressionText != null && ConfirmChoices.ExpressionText.TrimStart().StartsWith("${", StringComparison.InvariantCultureIgnoreCase))
{
// use ITemplate<ChocieSet> to bind (aka LG)
confirmChoices = await new ChoiceSet(ConfirmChoices.ExpressionText).BindAsync(dc, dc.State).ConfigureAwait(false);
}
else
{
// use expression to bind
confirmChoices = ConfirmChoices.TryGetValue(dc.State).Value;
}
}
else
{
confirmChoices = new ChoiceSet(new List<Choice>() { defaults.Item1, defaults.Item2 });
}

return confirmChoices;
}
}
}
2 changes: 1 addition & 1 deletion ...aries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/Dialogs/Microsoft.ChoiceInput.schema
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@
]
},
{
"$ref": "schema:#/definitions/equalsExpression"
"$ref": "schema:#/definitions/stringExpression"
}
]
},
Expand Down
2 changes: 1 addition & 1 deletion ...ries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/Dialogs/Microsoft.ConfirmInput.schema
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@
]
},
{
"$ref": "schema:#/definitions/equalsExpression"
"$ref": "schema:#/definitions/stringExpression"
}
]
}
Expand Down
Loading