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

OnReceive is only run if all relevant Middleware Complete #146

Merged
merged 23 commits into from
Feb 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1775422
DO NOT MERGE - Mostly done.
cleemullins Feb 16, 2018
fce5c60
Merge pull request #1 from Microsoft/master
Feb 16, 2018
c6096c3
Potential test for the exception handling.
Feb 16, 2018
971a58d
Improved test case.
Feb 16, 2018
bc7f4a0
DO NOT MERGE - Mostly done.
cleemullins Feb 16, 2018
5fe0db6
Bot runs OnReceive only if all relevant Middlware runs
cleemullins Feb 17, 2018
251ad1b
Merge pull request #1 from Microsoft/master
Feb 17, 2018
3d1f971
Merge remote-tracking branch 'origin/master' into CLM/PassExceptionsO…
Feb 17, 2018
ab244ba
Merged.
cleemullins Feb 17, 2018
a7c7047
Merge branch 'CLM/PassExceptionsOnTaskFaults'
Feb 17, 2018
800cb11
Changed to DataTestMethod for consistency due to MSTest V2
Feb 17, 2018
2a41914
Merge pull request #147 from RyanDawkins/master
cleemullins Feb 17, 2018
8095fec
Extracted QnAMaker logic from corresponding middleware
Feb 17, 2018
562f029
Merge pull request #148 from cod7alex/qnamaker-extract
cleemullins Feb 18, 2018
c0b6539
Updated QnAMaker to use v3 API
garypretty Feb 18, 2018
6aabdc1
Remove checks for undefined token types
dandriscoll Feb 18, 2018
0c46cab
Fix comments and move claim name constants into AuthenticationConstants
dandriscoll Feb 18, 2018
2e3f4d1
Remove trailing whitespace
dandriscoll Feb 18, 2018
cd1a296
Merge pull request #154 from dandriscoll/auth
cleemullins Feb 19, 2018
8d4aecc
Merge pull request #151 from garypretty/master
cleemullins Feb 19, 2018
756859b
DO NOT MERGE - Mostly done.
cleemullins Feb 16, 2018
208ed30
Bot runs OnReceive only if all relevant Middlware runs
cleemullins Feb 17, 2018
ea965aa
Merge branch 'CLM/RunOnReceiveAtTheEnd' of https://github.com/Microso…
cleemullins Feb 19, 2018
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
131 changes: 131 additions & 0 deletions libraries/Microsoft.Bot.Builder.Ai/QnAMaker.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using Newtonsoft.Json;
using System;
using System.Linq;
using System.Net.Http;
using System.Threading.Tasks;

namespace Microsoft.Bot.Builder.Ai
{
public class QnAMaker
{
public const string qnaMakerServiceEndpoint = "https://westus.api.cognitive.microsoft.com/qnamaker/v3.0/knowledgebases/";
public const string APIManagementHeader = "Ocp-Apim-Subscription-Key";
public const string JsonMimeType = "application/json";

private readonly HttpClient _httpClient;
private readonly QnAMakerOptions _options;
private readonly string _answerUrl;

public QnAMaker(QnAMakerOptions options, HttpClient httpClient)
{
_httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient));
_options = options ?? throw new ArgumentNullException(nameof(options));
if (string.IsNullOrEmpty(options.KnowledgeBaseId))
{
throw new ArgumentException(nameof(options.KnowledgeBaseId));
}

_answerUrl = $"{qnaMakerServiceEndpoint}{options.KnowledgeBaseId}/generateanswer";

if (_options.ScoreThreshold == 0)
{
_options.ScoreThreshold = 0.3F;
}

if (_options.Top == 0)
{
_options.Top = 1;
}

if (_options.StrictFilters == null)
{
_options.StrictFilters = new Metadata[] {};
}

if (_options.MetadataBoost == null)
{
_options.MetadataBoost = new Metadata[] { };
}
}

public async Task<QueryResult[]> GetAnswers(string question)
{
var request = new HttpRequestMessage(HttpMethod.Post, _answerUrl);

string jsonRequest = JsonConvert.SerializeObject(new
{
question,
top = _options.Top,
strictFilters = _options.StrictFilters,
metadataBoost = _options.MetadataBoost
}, Formatting.None);

var content = new StringContent(jsonRequest, System.Text.Encoding.UTF8, JsonMimeType);
content.Headers.Add(APIManagementHeader, _options.SubscriptionKey);

var response = await _httpClient.PostAsync(_answerUrl, content).ConfigureAwait(false);
if (response.IsSuccessStatusCode)
{
var jsonResponse = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
var results = JsonConvert.DeserializeObject<QueryResults>(jsonResponse);
foreach (var answer in results.Answers)
{
answer.Score = answer.Score / 100;
}

return results.Answers.Where(answer => answer.Score > _options.ScoreThreshold).ToArray();
}
return null;
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps a trace would be useful here - QnA service returning a failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@johnataylor

Agreed. That's part of a different PR (someone else did it) that's only showing up here because I rebased my PR against master to make merging cleaner.

}
}

public class QnAMakerOptions
{
public string SubscriptionKey { get; set; }
public string KnowledgeBaseId { get; set; }
public float ScoreThreshold { get; set; }
public int Top { get; set; }
public Metadata[] StrictFilters { get; set; }
public Metadata[] MetadataBoost { get; set; }
}

[Serializable]
public class Metadata
{
[JsonProperty(PropertyName = "name")]
public string Name { get; set; }

[JsonProperty(PropertyName = "value")]
public string Value { get; set; }
}

public class QueryResult
{
[JsonProperty("questions")]
public string[] Questions { get; set; }

[JsonProperty("answer")]
public string Answer { get; set; }

[JsonProperty("score")]
public float Score { get; set; }

[JsonProperty(PropertyName = "metadata")]
public Metadata[] Metadata { get; set; }

[JsonProperty(PropertyName = "source")]
public string Source { get; set; }

[JsonProperty(PropertyName = "qnaId")]
public int QnaId { get; set; }
}

public class QueryResults
{
[JsonProperty("answers")]
public QueryResult[] Answers { get; set; }
}
}
91 changes: 4 additions & 87 deletions libraries/Microsoft.Bot.Builder.Ai/QnAMakerMiddleware.cs
Original file line number Diff line number Diff line change
@@ -1,104 +1,21 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Linq;
using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.Bot.Builder.Middleware;
using Microsoft.Bot.Schema;
using Newtonsoft.Json;

namespace Microsoft.Bot.Builder.Ai
{

public class QueryResult
{
[JsonProperty("questions")]
public string[] Questions { get; set; }

[JsonProperty("answer")]
public string Answer { get; set; }

[JsonProperty("score")]
public float Score { get; set; }
}

public class QueryResults
{
[JsonProperty("answers")]
public QueryResult[] Answers { get; set; }
}


public class QnAMakerOptions
{
public string SubscriptionKey { get; set; }
public string KnowledgeBaseId { get; set; }
public float ScoreThreshold { get; set; }
public int Top { get; set; }
}

public class QnAMakerMiddleware : Middleware.IReceiveActivity
{
public const string qnaMakerServiceEndpoint = "https://westus.api.cognitive.microsoft.com/qnamaker/v2.0/knowledgebases/";
private readonly string _answerUrl;
private readonly QnAMakerOptions _options;
private readonly HttpClient _httpClient;

public const string APIManagementHeader = "Ocp-Apim-Subscription-Key";
public const string JsonMimeType = "application/json";
private readonly QnAMaker _qnaMaker;

public QnAMakerMiddleware(QnAMakerOptions options, HttpClient httpClient)
{

_options = options ?? throw new ArgumentNullException(nameof(options));
this._httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient));

if (string.IsNullOrEmpty(options.KnowledgeBaseId))
{
throw new ArgumentException(nameof(options.KnowledgeBaseId));
}

this._answerUrl = $"{qnaMakerServiceEndpoint}{options.KnowledgeBaseId}/generateanswer";

if (_options.ScoreThreshold == 0)
{
_options.ScoreThreshold = 0.3F;
}

if (_options.Top == 0)
{
_options.Top = 1;
}
}

public async Task<QueryResult[]> GetAnswers(string question)
{
var request = new HttpRequestMessage(HttpMethod.Post, this._answerUrl);

string jsonRequest = JsonConvert.SerializeObject(new
{
question,
top = this._options.Top
}, Formatting.None);

var content = new StringContent(jsonRequest, System.Text.Encoding.UTF8, JsonMimeType);
content.Headers.Add(APIManagementHeader, this._options.SubscriptionKey);

var response = await this._httpClient.PostAsync(this._answerUrl, content).ConfigureAwait(false);
if (response.IsSuccessStatusCode)
{
var jsonResponse = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
var results = JsonConvert.DeserializeObject<QueryResults>(jsonResponse);
foreach (var answer in results.Answers)
{
answer.Score = answer.Score / 100;
}

return results.Answers.Where(answer => answer.Score > this._options.ScoreThreshold).ToArray();
}
return null;
_qnaMaker = new QnAMaker(options, httpClient);
}

public async Task ReceiveActivity(IBotContext context, MiddlewareSet.NextDelegate next)
Expand All @@ -108,15 +25,15 @@ public async Task ReceiveActivity(IBotContext context, MiddlewareSet.NextDelegat
var messageActivity = context.Request.AsMessageActivity();
if (!string.IsNullOrEmpty(messageActivity.Text))
{
var results = await this.GetAnswers(messageActivity.Text.Trim()).ConfigureAwait(false);
var results = await _qnaMaker.GetAnswers(messageActivity.Text.Trim()).ConfigureAwait(false);
if (results.Any())
{
context.Reply(results.First().Answer);
}
}
}

await next().ConfigureAwait(false);
await next().ConfigureAwait(false);
}
}
}
20 changes: 14 additions & 6 deletions libraries/Microsoft.Bot.Builder/Bot.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.Linq;
using System.Net.Http;
using System.Threading.Tasks;
using Microsoft.Bot.Builder.Adapters;
using Microsoft.Bot.Schema;
Expand Down Expand Up @@ -56,12 +55,21 @@ private async Task RunPipeline(IBotContext context, Func<IBotContext, Task> proa
// Call any registered Middleware Components looking for ReceiveActivity()
if (context.Request != null)
{
await _middlewareSet.ReceiveActivity(context).ConfigureAwait(false);

// If the dev has registered a Receive Handler, call it.
if (this._onReceive != null)
bool didAllMiddlewareRun = await _middlewareSet.ReceiveActivityWithStatus(context).ConfigureAwait(false);
if (didAllMiddlewareRun)
{
// If the dev has registered a Receive Handler, call it.
if (_onReceive != null)
{
await _onReceive(context).ConfigureAwait(false);
}
}
else
{
await _onReceive(context).ConfigureAwait(false);
// One of the middleware instances did not call Next(). When this happens,
// by design, we do NOT call the OnReceive handler. This allows
// Middleware interceptors to be written that activly prevent certain
// Activites from being run.
}
}

Expand Down
Loading