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.

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

Jump to bottom

Added FxCop and Async analyzers to the QnAMaker project. #4274

Merged
merged 1 commit into from
Jul 15, 2020
Merged

Added FxCop and Async analyzers to the QnAMaker project. #4274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 29 additions & 29 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Dialogs/QnAMakerDialog.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ protected override async Task<bool> OnPreBubbleEventAsync(DialogContext dc, Dial
}

var suggestedQuestions = dc.State.GetValue<List<string>>($"this.suggestedQuestions");
if (suggestedQuestions != null && suggestedQuestions.Any(question => string.Compare(question, reply.Trim(), ignoreCase: true) == 0))
if (suggestedQuestions != null && suggestedQuestions.Any(question => string.Compare(question, reply.Trim(), StringComparison.OrdinalIgnoreCase) == 0))
{
// it matches one of the suggested actions, we like that.
return true;
Expand Down Expand Up @@ -439,7 +439,7 @@ protected virtual Task<QnAMakerOptions> GetQnAMakerOptionsAsync(DialogContext dc
/// <param name="dc">The <see cref="DialogContext"/> for the current turn of conversation.</param>
/// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
/// <remarks>If the task is successful, the result contains the response options to use.</remarks>
protected async virtual Task<QnADialogResponseOptions> GetQnAResponseOptionsAsync(DialogContext dc)
protected virtual async Task<QnADialogResponseOptions> GetQnAResponseOptionsAsync(DialogContext dc)
{
return new QnADialogResponseOptions
{
Expand All @@ -449,6 +449,31 @@ protected async virtual Task<QnADialogResponseOptions> GetQnAResponseOptionsAsyn
CardNoMatchResponse = await this.CardNoMatchResponse.BindAsync(dc).ConfigureAwait(false)
};
}

private static void ResetOptions(DialogContext dc, QnAMakerDialogOptions dialogOptions)
{
// Resetting context and QnAId
dialogOptions.QnAMakerOptions.QnAId = 0;
dialogOptions.QnAMakerOptions.Context = new QnARequestContext();

// -Check if previous context is present, if yes then put it with the query
// -Check for id if query is present in reverse index.
var previousContextData = ObjectPath.GetPathValue<Dictionary<string, int>>(dc.ActiveDialog.State, QnAContextData, new Dictionary<string, int>());
var previousQnAId = ObjectPath.GetPathValue<int>(dc.ActiveDialog.State, PreviousQnAId, 0);

if (previousQnAId > 0)
{
dialogOptions.QnAMakerOptions.Context = new QnARequestContext
{
PreviousQnAId = previousQnAId
};

if (previousContextData.TryGetValue(dc.Context.Activity.Text, out var currentQnAId))
{
dialogOptions.QnAMakerOptions.QnAId = currentQnAId;
}
}
}

private async Task<DialogTurnResult> CallGenerateAnswerAsync(WaterfallStepContext stepContext, CancellationToken cancellationToken)
{
Expand Down Expand Up @@ -485,7 +510,7 @@ private async Task<DialogTurnResult> CallGenerateAnswerAsync(WaterfallStepContex
// Get filtered list of the response that support low score variation criteria.
response.Answers = qnaClient.GetLowScoreVariation(response.Answers);

if (response.Answers.Count() > 1 && isActiveLearningEnabled)
if (response.Answers.Length > 1 && isActiveLearningEnabled)
gabog marked this conversation as resolved.
Show resolved Hide resolved
{
var suggestedQuestions = new List<string>();
foreach (var qna in response.Answers)
Expand Down Expand Up @@ -516,31 +541,6 @@ private async Task<DialogTurnResult> CallGenerateAnswerAsync(WaterfallStepContex
return await stepContext.NextAsync(result, cancellationToken).ConfigureAwait(false);
}

private void ResetOptions(DialogContext dc, QnAMakerDialogOptions dialogOptions)
{
// Resetting context and QnAId
dialogOptions.QnAMakerOptions.QnAId = 0;
dialogOptions.QnAMakerOptions.Context = new QnARequestContext();

// -Check if previous context is present, if yes then put it with the query
// -Check for id if query is present in reverse index.
var previousContextData = ObjectPath.GetPathValue<Dictionary<string, int>>(dc.ActiveDialog.State, QnAContextData, new Dictionary<string, int>());
var previousQnAId = ObjectPath.GetPathValue<int>(dc.ActiveDialog.State, PreviousQnAId, 0);

if (previousQnAId > 0)
{
dialogOptions.QnAMakerOptions.Context = new QnARequestContext
{
PreviousQnAId = previousQnAId
};

if (previousContextData.TryGetValue(dc.Context.Activity.Text, out var currentQnAId))
{
dialogOptions.QnAMakerOptions.QnAId = currentQnAId;
}
}
}

private async Task<DialogTurnResult> CallTrainAsync(WaterfallStepContext stepContext, CancellationToken cancellationToken)
{
var dialogOptions = ObjectPath.GetPathValue<QnAMakerDialogOptions>(stepContext.ActiveDialog.State, Options);
Expand Down Expand Up @@ -613,7 +613,7 @@ private async Task<DialogTurnResult> CheckForMultiTurnPromptAsync(WaterfallStepC

var answer = response.First();

if (answer.Context != null && answer.Context.Prompts.Count() > 0)
if (answer.Context != null && answer.Context.Prompts.Length > 0)
{
var previousContextData = ObjectPath.GetPathValue(stepContext.ActiveDialog.State, QnAContextData, new Dictionary<string, int>());
var previousQnAId = ObjectPath.GetPathValue<int>(stepContext.ActiveDialog.State, PreviousQnAId, 0);
Expand Down
23 changes: 15 additions & 8 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Microsoft.Bot.Builder.AI.QnA.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<DebugType>Full</DebugType>
<DebugSymbols>true</DebugSymbols>
</PropertyGroup>

<ItemGroup>
<Content Include="**/*.dialog" />
<Content Include="**/*.lg" />
Expand All @@ -31,20 +31,27 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="AsyncUsageAnalyzers" Version="1.0.0-alpha003" PrivateAssets="all" />
<PackageReference Include="AdaptiveExpressions" Condition=" '$(IsBuildServer)' == '' " Version="$(LocalPackageVersion)" />
<PackageReference Include="AdaptiveExpressions" Condition=" '$(IsBuildServer)' != '' " Version="$(ReleasePackageVersion)" />
<PackageReference Include="AsyncUsageAnalyzers" Version="1.0.0-alpha003">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.Bot.Configuration" Condition=" '$(IsBuildServer)' == '' " Version="$(LocalPackageVersion)" />
<PackageReference Include="Microsoft.Bot.Configuration" Condition=" '$(IsBuildServer)' != '' " Version="$(ReleasePackageVersion)" />
<PackageReference Include="Microsoft.Bot.Builder.Dialogs.Declarative" Condition=" '$(IsBuildServer)' == '' " Version="$(LocalPackageVersion)" />
<PackageReference Include="Microsoft.Bot.Builder.Dialogs.Declarative" Condition=" '$(IsBuildServer)' != '' " Version="$(ReleasePackageVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="3.0.0">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\AdaptiveExpressions\AdaptiveExpressions.csproj" />
<ProjectReference Include="..\Microsoft.Bot.Builder.Dialogs.Declarative\Microsoft.Bot.Builder.Dialogs.Declarative.csproj" />
<ProjectReference Include="..\Microsoft.Bot.Builder\Microsoft.Bot.Builder.csproj" />
<ProjectReference Include="..\Microsoft.Bot.Configuration\Microsoft.Bot.Configuration.csproj" />
</ItemGroup>
</Project>
<ProjectReference Include="..\AdaptiveExpressions\AdaptiveExpressions.csproj" />
<ProjectReference Include="..\Microsoft.Bot.Builder.Dialogs.Declarative\Microsoft.Bot.Builder.Dialogs.Declarative.csproj" />
<ProjectReference Include="..\Microsoft.Bot.Builder\Microsoft.Bot.Builder.csproj" />
<ProjectReference Include="..\Microsoft.Bot.Configuration\Microsoft.Bot.Configuration.csproj" />
</ItemGroup>
</Project>
2 changes: 2 additions & 0 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Models/FeedbackRecords.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ public class FeedbackRecords
/// List of feedback records.
/// </value>
[JsonProperty("feedbackRecords")]
#pragma warning disable CA1819 // Properties should not return arrays (we can't change this without breaking binary compat)
public FeedbackRecord[] Records { get; set; }
#pragma warning restore CA1819 // Properties should not return arrays
}
}
2 changes: 2 additions & 0 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Models/Metadata.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ namespace Microsoft.Bot.Builder.AI.QnA
/// Represents the Metadata object sent as part of QnA Maker requests.
/// </summary>
[Serializable]
#pragma warning disable CA1724 // Type names should not match namespaces (we can't change this without breaking binary compat)
public class Metadata
#pragma warning restore CA1724 // Type names should not match namespaces
{
/// <summary>
/// Gets or sets the name for the Metadata property.
Expand Down
10 changes: 8 additions & 2 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Models/QnAMakerTraceInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ public class QnAMakerTraceInfo
/// Results that QnAMaker returned.
/// </value>
[JsonProperty("queryResults")]
#pragma warning disable CA1819 // Properties should not return arrays (we can't change this without breaking binary compat)
public QueryResult[] QueryResults { get; set; }
#pragma warning restore CA1819 // Properties should not return arrays

/// <summary>
/// Gets or sets iD of the Knowledgebase that is being used.
Expand Down Expand Up @@ -66,8 +68,10 @@ public class QnAMakerTraceInfo
/// The filters used to return answers that have the specified metadata.
/// </value>
[JsonProperty("strictFilters")]
public Metadata[] StrictFilters { get; set; }

#pragma warning disable CA1819 // Properties should not return arrays (we can't change this without breaking binary compat)
public Metadata[] StrictFilters { get; set; }
#pragma warning restore CA1819 // Properties should not return arrays

/// <summary>
/// Gets or sets context for multi-turn responses.
/// </summary>
Expand Down Expand Up @@ -112,6 +116,8 @@ public class QnAMakerTraceInfo
/// </value>
[Obsolete("This property is no longer used and will be ignored")]
[JsonIgnore]
#pragma warning disable CA1819 // Properties should not return arrays (this property is obsolete and we won't change it)
public Metadata[] MetadataBoost { get; set; }
#pragma warning restore CA1819 // Properties should not return arrays
}
}
2 changes: 2 additions & 0 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Models/QnAResponseContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public class QnAResponseContext
/// The QnA prompts array.
/// </value>
[JsonProperty(PropertyName = "prompts")]
#pragma warning disable CA1819 // Properties should not return arrays (we can't change this without breaking binary compat)
public QnaMakerPrompt[] Prompts { get; set; }
#pragma warning restore CA1819 // Properties should not return arrays
}
}
4 changes: 4 additions & 0 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Models/QueryResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ public class QueryResult
/// The list of questions indexed in the QnA Service for the given answer.
/// </value>
[JsonProperty("questions")]
#pragma warning disable CA1819 // Properties should not return arrays (we can't change this without breaking binary compat)
public string[] Questions { get; set; }
#pragma warning restore CA1819 // Properties should not return arrays

/// <summary>
/// Gets or sets the answer text.
Expand Down Expand Up @@ -46,7 +48,9 @@ public class QueryResult
/// Metadata that is associated with the answer.
/// </value>
[JsonProperty(PropertyName = "metadata")]
#pragma warning disable CA1819 // Properties should not return arrays (we can't change this without breaking binary compat)
public Metadata[] Metadata { get; set; }
#pragma warning restore CA1819 // Properties should not return arrays

/// <summary>
/// Gets or sets the source from which the QnA was extracted.
Expand Down
2 changes: 2 additions & 0 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Models/QueryResults.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ public class QueryResults
/// sorted in decreasing order of ranking score.
/// </value>
[JsonProperty("answers")]
#pragma warning disable CA1819 // Properties should not return arrays (we can't change this without breaking binary compat)
public QueryResult[] Answers { get; set; }
#pragma warning restore CA1819 // Properties should not return arrays

/// <summary>
/// Gets or sets a value indicating whether gets or set for the active learning enable flag.
Expand Down
2 changes: 2 additions & 0 deletions libraries/Microsoft.Bot.Builder.AI.QnA/Models/RankerTypes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ namespace Microsoft.Bot.Builder.AI.QnA
/// <summary>
/// Enumeration of types of ranking.
/// </summary>
#pragma warning disable CA1052 // Static holder types should be Static or NotInheritable (we can't change this without breaking binary compat)
public class RankerTypes
#pragma warning restore CA1052 // Static holder types should be Static or NotInheritable
{
/// <summary>
/// Default Ranker Behaviour. i.e. Ranking based on Questions and Answer.
Expand Down
15 changes: 12 additions & 3 deletions libraries/Microsoft.Bot.Builder.AI.QnA/QnAMaker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Net.Http;
using System.Threading;
Expand Down Expand Up @@ -67,7 +68,7 @@ public QnAMaker(QnAMakerEndpoint endpoint, QnAMakerOptions options, HttpClient h
throw new ArgumentException(nameof(endpoint.EndpointKey));
}

if (_endpoint.Host.EndsWith("v2.0") || _endpoint.Host.EndsWith("v3.0"))
if (_endpoint.Host.EndsWith("v2.0", StringComparison.Ordinal) || _endpoint.Host.EndsWith("v3.0", StringComparison.Ordinal))
{
throw new NotSupportedException("v2.0 and v3.0 of QnA Maker service is no longer supported in the QnA Maker.");
}
Expand Down Expand Up @@ -109,10 +110,13 @@ public QnAMaker(QnAMakerEndpoint endpoint, QnAMakerOptions options = null, HttpC
/// If null, a default client is used for this instance.</param>
/// <param name="telemetryClient">The IBotTelemetryClient used for logging telemetry events.</param>
/// <param name="logPersonalInformation">Set to true to include personally identifiable information in telemetry events.</param>
[Obsolete("Constructor is deprecated, please use QnAMaker(QnAMakerEndpoint endpoint, QnAMakerOptions options, HttpClient httpClient).")]
#pragma warning disable CS0618 // Type or member is obsolete, this is here only for backward compat and should be removed when QnAMakerService is removed.
public QnAMaker(QnAMakerService service, QnAMakerOptions options, HttpClient httpClient, IBotTelemetryClient telemetryClient, bool logPersonalInformation = false)
: this(new QnAMakerEndpoint(service), options, httpClient, telemetryClient, logPersonalInformation)
{
}
#pragma warning restore CS0618 // Type or member is obsolete

/// <summary>
/// Initializes a new instance of the <see cref="QnAMaker"/> class.
Expand All @@ -121,10 +125,13 @@ public QnAMaker(QnAMakerService service, QnAMakerOptions options, HttpClient htt
/// <param name="options">The options for the QnA Maker knowledge base.</param>
/// <param name="httpClient">An alternate client with which to talk to QnAMaker.
/// If null, a default client is used for this instance.</param>
[Obsolete("Constructor is deprecated, please use QnAMaker(QnAMakerEndpoint endpoint, QnAMakerOptions options, HttpClient httpClient).")]
#pragma warning disable CS0618 // Type or member is obsolete, this is here only for backward compat and should be removed when QnAMakerService is removed.
public QnAMaker(QnAMakerService service, QnAMakerOptions options = null, HttpClient httpClient = null)
: this(new QnAMakerEndpoint(service), options, httpClient, null)
{
}
#pragma warning restore CS0618 // Type or member is obsolete

/// <summary>
/// Gets the <see cref="HttpClient"/> to be used when calling the QnA Maker API.
Expand Down Expand Up @@ -198,7 +205,7 @@ public async Task<QueryResults> GetAnswersRawAsync(

if (turnContext.Activity == null)
{
throw new ArgumentNullException(nameof(turnContext.Activity));
throw new ArgumentException($"The {nameof(turnContext.Activity)} property for {nameof(turnContext)} can't be null.", nameof(turnContext));
}

var messageActivity = turnContext.Activity.AsMessageActivity();
Expand Down Expand Up @@ -272,7 +279,9 @@ protected virtual async Task OnQnaResultsAsync(
/// <param name="cancellationToken">A cancellation token that can be used by other objects or threads to receive notice of cancellation.</param>
/// additionalProperties
/// <returns>A tuple of Properties and Metrics that will be sent to the IBotTelemetryClient.TrackEvent method for the QnAMessage event. The properties and metrics returned the standard properties logged with any properties passed from the GetAnswersAsync method.</returns>
#pragma warning disable CA1801 // Review unused parameters (we can't remove cancellationToken without breaking binary compat)
protected Task<(Dictionary<string, string> Properties, Dictionary<string, double> Metrics)> FillQnAEventAsync(QueryResult[] queryResults, ITurnContext turnContext, Dictionary<string, string> telemetryProperties = null, Dictionary<string, double> telemetryMetrics = null, CancellationToken cancellationToken = default(CancellationToken))
#pragma warning restore CA1801 // Review unused parameters
{
var properties = new Dictionary<string, string>();
var metrics = new Dictionary<string, double>();
Expand Down Expand Up @@ -301,7 +310,7 @@ protected virtual async Task OnQnaResultsAsync(
{
var queryResult = queryResults[0];
properties.Add(QnATelemetryConstants.MatchedQuestionProperty, JsonConvert.SerializeObject(queryResult.Questions));
properties.Add(QnATelemetryConstants.QuestionIdProperty, queryResult.Id.ToString());
properties.Add(QnATelemetryConstants.QuestionIdProperty, queryResult.Id.ToString(CultureInfo.InvariantCulture));
properties.Add(QnATelemetryConstants.AnswerProperty, queryResult.Answer);
metrics.Add(QnATelemetryConstants.ScoreProperty, queryResult.Score);
properties.Add(QnATelemetryConstants.ArticleFoundProperty, "true");
Expand Down
6 changes: 5 additions & 1 deletion libraries/Microsoft.Bot.Builder.AI.QnA/QnAMakerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public QnAMakerOptions()
public QnARequestContext Context { get; set; }

/// <summary>
/// Gets or sets QnA Id of the current question asked (if avaliable).
/// Gets or sets QnA Id of the current question asked (if availble).
/// </summary>
/// <value>
/// Id of the current question asked.
Expand All @@ -77,7 +77,9 @@ public QnAMakerOptions()
/// An array of <see cref="Metadata"/>.
/// </value>
[JsonProperty("strictFilters")]
#pragma warning disable CA1819 // Properties should not return arrays (we can't change this without breaking binary compat)
public Metadata[] StrictFilters { get; set; }
#pragma warning restore CA1819 // Properties should not return arrays

/// <summary>
/// Gets or sets the <see cref="Metadata"/> collection to be sent when calling QnA Maker to boost results.
Expand All @@ -87,7 +89,9 @@ public QnAMakerOptions()
/// </value>
[Obsolete("This property is no longer used and will be ignored")]
[JsonIgnore]
#pragma warning disable CA1819 // Properties should not return arrays (property is obsolete, we won't change it)
public Metadata[] MetadataBoost { get; set; }
#pragma warning restore CA1819 // Properties should not return arrays

/// <summary>
/// Gets or sets a value indicating whether to call test or prod environment of knowledge base to be called.
Expand Down
Loading