Skip to content
This repository was archived by the owner on Jan 5, 2026. It is now read-only.

Refactor of Microsoft.Bot.Builder.Azure.Queues. #4692

Merged
mrivera-ms merged 5 commits into from
Oct 8, 2020
Merged

Refactor of Microsoft.Bot.Builder.Azure.Queues. #4692

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cd143b8
Refactor of Microsoft.Bot.Builder.Azure.Queues.
mrivera-ms Sep 24, 2020
5bad8d6
Moving QueueStorage to TurnState.
mrivera-ms Sep 26, 2020
7e9f010
Add visibility timeout.
mrivera-ms Sep 26, 2020
b349eeb
Changes requested by peer review.
mrivera-ms Oct 7, 2020
47a640e
Remove references to Dialogs.Declarative.
mrivera-ms Oct 7, 2020
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
21 changes: 11 additions & 10 deletions ...Bot.Builder.Azure.Queues/QueuesStorage.cs → ...Builder.Azure.Queues/AzureQueueStorage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ namespace Microsoft.Bot.Builder.Azure.Queues
/// <summary>
/// Service used to add messages to an Azure.Storage.Queues.
/// </summary>
public class QueuesStorage
public class AzureQueueStorage : QueueStorage
{
private JsonSerializerSettings _jsonSettings;
private readonly JsonSerializerSettings _jsonSettings;
private bool _createQueueIfNotExists = true;
private readonly QueueClient _queueClient;

/// <summary>
/// Initializes a new instance of the <see cref="QueuesStorage"/> class.
/// Initializes a new instance of the <see cref="AzureQueueStorage"/> class.
/// </summary>
/// <param name="queuesStorageConnectionString">Azure Storage connection string.</param>
/// <param name="queueName">Name of the storage queue where entities will be queued.</param>
Expand All @@ -31,7 +31,7 @@ public class QueuesStorage
/// <para>jsonSerializer.TypeNameHandling = TypeNameHandling.None.</para>
/// <para>jsonSerializer.NullValueHandling = NullValueHandling.Ignore.</para>
/// </param>
public QueuesStorage(string queuesStorageConnectionString, string queueName, JsonSerializerSettings jsonSerializerSettings = null)
public AzureQueueStorage(string queuesStorageConnectionString, string queueName, JsonSerializerSettings jsonSerializerSettings = null)
{
if (string.IsNullOrEmpty(queuesStorageConnectionString))
{
Expand All @@ -44,16 +44,17 @@ public QueuesStorage(string queuesStorageConnectionString, string queueName, Jso
}

_jsonSettings = jsonSerializerSettings ?? new JsonSerializerSettings
{
TypeNameHandling = TypeNameHandling.None,
NullValueHandling = NullValueHandling.Ignore
};
{
TypeNameHandling = TypeNameHandling.None,
NullValueHandling = NullValueHandling.Ignore
};

_queueClient = new QueueClient(queuesStorageConnectionString, queueName);
}

/// <summary>
/// Queue and Activity, with option in the Activity.Value to Azure.Storage.Queues.
/// Queue an Activity to an Azure.Storage.Queues.QueueClient. The visibility timeout specifies how long the message should be invisible
/// to Dequeue and Peek operations. The message content must be a UTF-8 encoded string that is up to 64KB in size.
/// </summary>
/// <param name="activity">This is expected to be an <see cref="Activity"/> retrieved from a call to
/// activity.GetConversationReference().GetContinuationActivity(). This enables restarting the conversation
Expand All @@ -62,7 +63,7 @@ public QueuesStorage(string queuesStorageConnectionString, string queueName, Jso
/// <param name="timeToLive">Specifies the time-to-live interval for the message.</param>
/// <param name="cancellationToken">Cancellation token for the async operation.</param>
/// <returns><see cref="SendReceipt"/> as a Json string, from the QueueClient SendMessageAsync operation.</returns>
public async Task<string> QueueActivityAsync(Activity activity, TimeSpan? visibilityTimeout = null, TimeSpan? timeToLive = null, CancellationToken cancellationToken = default)
public override async Task<string> QueueActivityAsync(Activity activity, TimeSpan? visibilityTimeout = null, TimeSpan? timeToLive = null, CancellationToken cancellationToken = default)
{
if (_createQueueIfNotExists)
{
Expand Down
32 changes: 5 additions & 27 deletions ...Azure.Queues/ContinueConversationLater.cs → ...tive/Actions/ContinueConversationLater.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
using System.Threading;
using System.Threading.Tasks;
using AdaptiveExpressions.Properties;
using Microsoft.Bot.Builder.Dialogs;
using Newtonsoft.Json;

namespace Microsoft.Bot.Builder.Azure.Queues
namespace Microsoft.Bot.Builder.Dialogs.Adaptive.Actions
{
/// <summary>
/// Action which schedules a conversation to be continued later by writing a EventActivity(Name=ContinueConversation) to a Azure Storage queue.
Comment thread
gabog marked this conversation as resolved.
Outdated

@chrimc62 chrimc62 Oct 7, 2020 •
edited by mrivera-ms
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

a [](start = 79, length = 1)

an #Resolved

Expand All @@ -32,7 +31,7 @@ public class ContinueConversationLater : Dialog
/// The Kind name for this dialog.
/// </summary>
[JsonProperty("$kind")]
public const string Kind = "AzureQueues.ContinueConversationLater";
public const string Kind = "Microsoft.ContinueConversationLater";

@gabog gabog Oct 7, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure if we still do this, but I think we need to update test.schema with the new name (I don't know if test.schema gets regenerated on the build server but the copy in source control has the old name)

@mrivera-ms mrivera-ms Oct 7, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you mean the schema in libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Schemas/Actions? If so, that change is included in this PR. I have not found any other references in the source. Please let me know.

In reply to: 501018849 [](ancestors = 501018849)

@gabog gabog Oct 7, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I searched on my local copy of schema.tests and I see a reference to the old name, I do see it too on your branch here, here and in a couple of other places.

It seems that tests.schema gets regenerated by the unit tests so I am not sure if this is relevant or not (if tests.schema is regenerated every time we may want to added to the gitignore, would this be the case @chrimc62?)


/// <summary>
/// Initializes a new instance of the <see cref="ContinueConversationLater"/> class.
Expand Down Expand Up @@ -74,23 +73,6 @@ public ContinueConversationLater([CallerFilePath] string callerPath = "", [Calle
[JsonProperty("value")]
public ValueExpression Value { get; set; }

/// <summary>
/// Gets or sets the connectionString for the azure storage queue to use.
/// </summary>
/// <value>
/// The connectionString for the azure storage queue to use.
/// </value>
/// <example>'=settings.ConnectionString'.</example>
[JsonProperty("connectionString")]
public StringExpression ConnectionString { get; set; }

/// <summary>
/// Gets or sets the name of the queue to use.
/// </summary>
/// <value>default is 'activities'.</value>
[JsonProperty("queueName")]
public StringExpression QueueName { get; set; } = "activities";

/// <inheritdoc/>
public override async Task<DialogTurnResult> BeginDialogAsync(DialogContext dc, object options = null, CancellationToken cancellationToken = default(CancellationToken))
{
Expand All @@ -105,12 +87,11 @@ public ContinueConversationLater([CallerFilePath] string callerPath = "", [Calle
}

var dateString = Date.GetValue(dc.State);
DateTime date;
if (!DateTime.TryParse(dateString, out date))
if (!DateTime.TryParse(dateString, out var date))
{
throw new ArgumentException($"{nameof(Date)} is invalid");
}

date = date.ToUniversalTime();
if (date <= DateTime.UtcNow)
{
Expand All @@ -124,10 +105,7 @@ public ContinueConversationLater([CallerFilePath] string callerPath = "", [Calle
var visibility = date - DateTime.UtcNow;
var ttl = visibility + TimeSpan.FromMinutes(2);

var queueName = QueueName.GetValue(dc.State);
var connectionString = ConnectionString.GetValue(dc.State);

var queueStorage = new QueuesStorage(connectionString, queueName);
var queueStorage = dc.Context.TurnState.Get<QueueStorage>() ?? throw new NullReferenceException("Unable to locate QueueStorage in HostContext");
var receipt = await queueStorage.QueueActivityAsync(activity, visibility, ttl, cancellationToken).ConfigureAwait(false);

// return the receipt as the result.
Expand Down
17 changes: 0 additions & 17 deletions ...reQueues.ContinueConversationLater.schema → ...icrosoft.ContinueConversationLater.schema
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,6 @@
"=addHours(utcNow(), 1)"
]
},
"connectionString": {
"$ref": "schema:#/definitions/stringExpression",
"title": "Connection String",
"description": "Connection string for connecting to an Azure Storage account.",
"examples": [
"=settings.connectionString"
]
},
"queueName": {
"$ref": "schema:#/definitions/stringExpression",
"title": "Queue Name",
"description": "Name of the queue that your azure function is bound to.",
"examples": [
"activities"
],
"default": "activities"

@chrimc62 chrimc62 Oct 7, 2020 •
edited by mrivera-ms
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It looks like the class was changed to match the schema name is good, but if we have shipped this it is a breaking change. Have we shipped it? #Resolved

@mrivera-ms mrivera-ms Oct 7, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It shipped as a preview, so we should be safe to change it.

In reply to: 501232641 [](ancestors = 501232641)

},
"value": {
"$ref": "schema:#/definitions/valueExpression",
"title": "Value",
Expand Down
24 changes: 24 additions & 0 deletions libraries/Microsoft.Bot.Builder/QueueStorage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
using System;
Comment thread
gabog marked this conversation as resolved.
Outdated
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Bot.Schema;

namespace Microsoft.Bot.Builder
{
/// <summary>
/// A base class for enqueueing an Activity for later processing.
/// </summary>
public abstract class QueueStorage
{
/// <summary>
/// Enqueues an Activity for later processing. The visibility timeout specifies how long the message should be invisible
/// to Dequeue and Peek operations. The message content must be a UTF-8 encoded string that is up to 64KB in size.
/// </summary>
/// <param name="activity">The <see cref="Activity"/> to be queued for later processing.</param>
/// <param name="visibilityTimeout"> Visibility timeout. Optional with a default value of 0. Cannot be larger than 7 days. </param>
/// <param name="timeToLive">Specifies the time-to-live interval for the message.</param>
/// <param name="cancellationToken">Cancellation token for the async operation.</param>
/// <returns>A result string.</returns>
public abstract Task<string> QueueActivityAsync(Activity activity, TimeSpan? visibilityTimeout = null, TimeSpan? timeToLive = null, CancellationToken cancellationToken = default);
}
}
7 changes: 5 additions & 2 deletions tests/Microsoft.Bot.Builder.Azure.Tests/AzureQueueTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.Bot.Builder.Adapters;
using Microsoft.Bot.Builder.Azure.Queues;
using Microsoft.Bot.Builder.Dialogs;
using Microsoft.Bot.Builder.Dialogs.Adaptive.Actions;
using Microsoft.Bot.Builder.Tests;
using Microsoft.Bot.Schema;
using Newtonsoft.Json;
Expand Down Expand Up @@ -40,13 +41,15 @@ public async Task ContinueConversationLaterTests()
.UseStorage(new MemoryStorage())
.UseBotState(new ConversationState(new MemoryStorage()), new UserState(new MemoryStorage()));

var queueStorage = new AzureQueueStorage(ConnectionString, queueName);
var dm = new DialogManager(new ContinueConversationLater()
{
ConnectionString = ConnectionString,
QueueName = queueName,
Date = "=addSeconds(utcNow(), 2)",
Value = "foo"
});

dm.InitialTurnState.Set<QueueStorage>(queueStorage);

@chrimc62 chrimc62 Oct 7, 2020 •
edited by mrivera-ms
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are there any samples which did this in the old way? Do we need a sample? #Resolved

@mrivera-ms mrivera-ms Oct 7, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There are no samples to update with this change. We do have a plan to add this to a sample later.

In reply to: 501233216 [](ancestors = 501233216)


await new TestFlow((TestAdapter)adapter, dm.OnTurnAsync)
.Send("hi")
.StartTestAsync();
Expand Down