[Azure] Fix 'PartitionKey value must be supplied for this operation' error

[JuanAr]

Description

This PR will fix the Issue #1407.

Changes made

• Add PartitionKey field to the CosmosDbStorageOptions
• Modify the DeleteAsync method to generate the RequestOptions with the PartitionKey provided by the CosmosDbStorageOptions
• Pass the options optional parameter to the DeleteDocumentAsync method.

Testing

1. Connect your bot to a CosmosDB

2. Add the following on the main bot class:

private const string CosmosServiceEndpoint = "Endpoint";
private const string CosmosDBKey = "YourDBKey";
private const string CosmosDBDatabaseName = "bot-cosmos-sql-db";
private const string CosmosDBCollectionName = "bot-storage";
private const string CosmosDBPartitionKey = "YourPartitionKey";

// Replaces Memory Storage with reference to Cosmos DB.
private static readonly CosmosDbStorage _myStorage = new CosmosDbStorage(new CosmosDbStorageOptions
{
    PartitionKey = CosmosDBPartitionKey,
    AuthKey = CosmosDBKey,
    CollectionId = CosmosDBCollectionName,
    CosmosDBEndpoint = new Uri(CosmosServiceEndpoint),
    DatabaseId = CosmosDBDatabaseName,
});

and

string[] utteranceList = { "id-to-delete" };
await _myStorage.DeleteAsync(utteranceList, cancellationToken);

On the EmptyBot, it should look something like this.

[coveralls]

Pull Request Test Coverage Report for Build 53295

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 52 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.09%) to 76.031%

---

Files with Coverage Reduction	New Missed Lines	%
/libraries/Microsoft.Bot.Builder.Azure/CosmosDbStorageOptions.cs	2	75.0%
/libraries/Microsoft.Bot.Builder.Azure/CosmosDbStorage.cs	50	51.63%

Totals
Change from base Build 53133:	-0.09%
Covered Lines:	4368
Relevant Lines:	5745

---

💛 - Coveralls

[cleemullins]

What will the impact of this change be on folks that have data already stored in CosmosDB and upgrade to this code?

That is, is the behavior of "PartitionKey=Null" the same as the 4.3 code behavior?

[gasper-az]

What will the impact of this change be on folks that have data already stored in CosmosDB and upgrade to this code?

That is, is the behavior of "PartitionKey=Null" the same as the 4.3 code behavior?

@cleemullins
The only major change we've done was adding to the CosmosDbStorageOptions the PartitionKey property. The CosmosDbStorageOptions class doesn’t have a constructor which means the implementations that already exist would not be affected by the changes made.

This property might be configured only in cases where the user needs to perform a delete operation that requires a PartitionKey. It would be necessary to add it to the CosmosDbStorageOptions instance that initializes the CosmosDbStorage with the PartitionKey value.

[cleemullins]

Build is failing. Here is the Test Error:

Failed   DeleteAsyncFromPartitionedCollectionWithoutPartitionKey
Error Message:
 Test method Microsoft.Bot.Builder.Azure.Tests.CosmosDbStorageTests.DeleteAsyncFromPartitionedCollectionWithoutPartitionKey threw exception: 
System.Net.Http.HttpRequestException: No connection could be made because the target machine actively refused it ---> System.Net.Sockets.SocketException: No connection could be made because the target machine actively refused it
Stack Trace:
    at System.Net.Http.ConnectHelper.ConnectAsync(String host, Int32 port, CancellationToken cancellationToken)
--- End of inner exception stack trace ---
    at System.Net.Http.ConnectHelper.ConnectAsync(String host, Int32 port, CancellationToken cancellationToken)
   at System.Threading.Tasks.ValueTask`1.get_Result()
   at System.Net.Http.HttpConnectionPool.CreateConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Threading.Tasks.ValueTask`1.get_Result()
   at System.Net.Http.HttpConnectionPool.WaitForCreatedConnectionAsync(ValueTask`1 creationTask)
   at System.Threading.Tasks.ValueTask`1.get_Result()
   at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at Microsoft.Azure.Documents.Client.DocumentClient.HttpRequestMessageHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.FinishSendAsyncBuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts)
   at Microsoft.Azure.Documents.Client.GatewayServiceConfigurationReader.GetDatabaseAccountAsync(Uri serviceEndpoint)
   at Microsoft.Azure.Documents.Routing.GlobalEndpointManager.GetDatabaseAccountFromAnyLocationsAsync(Uri defaultEndpoint, IList`1 locations, Func`2 getDatabaseAccountFn)
   at Microsoft.Azure.Documents.Client.GatewayServiceConfigurationReader.InitializeReaderAsync()
   at Microsoft.Azure.Documents.Client.DocumentClient.InitializeGatewayConfigurationReader()
   at Microsoft.Azure.Documents.Client.DocumentClient.GetInitializationTask()
   at Microsoft.Azure.Documents.Client.DocumentClient.EnsureValidClientAsync()
   at Microsoft.Azure.Documents.Client.DocumentClient.ReadDatabasePrivateAsync(String databaseLink, RequestOptions options, IDocumentClientRetryPolicy retryPolicyInstance)
   at Microsoft.Azure.Documents.BackoffRetryUtility`1.<>c__DisplayClass1_0.<<ExecuteAsync>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.Azure.Documents.BackoffRetryUtility`1.ExecuteRetryAsync(Func`1 callbackMethod, Func`3 callShouldRetry, Func`1 inBackoffAlternateCallbackMethod, TimeSpan minBackoffForInBackoffCallback, CancellationToken cancellationToken, Action`1 preRetryCallback)
   at Microsoft.Azure.Documents.ShouldRetryResult.ThrowIfDoneTrying(ExceptionDispatchInfo capturedException)
   at Microsoft.Azure.Documents.BackoffRetryUtility`1.ExecuteRetryAsync(Func`1 callbackMethod, Func`3 callShouldRetry, Func`1 inBackoffAlternateCallbackMethod, TimeSpan minBackoffForInBackoffCallback, CancellationToken cancellationToken, Action`1 preRetryCallback)
   at Microsoft.Azure.Documents.BackoffRetryUtility`1.ExecuteAsync(Func`1 callbackMethod, IRetryPolicy retryPolicy, CancellationToken cancellationToken, Action`1 preRetryCallback)
   at Microsoft.Azure.Documents.Client.DocumentClient.CreateDatabaseIfNotExistsPrivateAsync(Database database, RequestOptions options)
   at Microsoft.Bot.Builder.Azure.Tests.CosmosDbStorageTests.CreateCosmosDbWithPartitionedCollection(String partitionKey) in D:\a\1\s\tests\Microsoft.Bot.Builder.Azure.Tests\CosmosDbStorageTests.cs:line 488
   at Microsoft.Bot.Builder.Azure.Tests.CosmosDbStorageTests.DeleteAsyncFromPartitionedCollectionWithoutPartitionKey() in D:\a\1\s\tests\Microsoft.Bot.Builder.Azure.Tests\CosmosDbStorageTests.cs:line 461

Results File: D:\a\_temp\VssAdministrator_fv-az381_2019-03-26_22_14_50.trx

[gasper-az]

@cleemullins, we are currently checking why the Test method is making the build fail.

[gasper-az]

@cleemullins Build fixed

[cleemullins]

LGTM. The tests look complete, and the fix seems sensible.

[cleemullins]

So I'm a bit confused. I see _partitionKey has been added which is great.

... however it's only consumed on "delete".
Read/Write don't seem to use this field at all.

Is this correct and by design for CosmosDB, or is this an oversight in the PR?

[cleemullins]

No issues found in Microsoft.Bot.Builder.dll
No issues found in Microsoft.Bot.Builder.AI.Luis.dll
No issues found in Microsoft.Bot.Builder.AI.QnA.dll
No issues found in Microsoft.Bot.Builder.ApplicationInsights.dll
No issues found in Microsoft.Bot.Builder.Azure.dll
No issues found in Microsoft.Bot.Builder.Dialogs.dll
No issues found in Microsoft.Bot.Builder.TemplateManager.dll
No issues found in Microsoft.Bot.Configuration.dll
No issues found in Microsoft.Bot.Connector.dll
No issues found in Microsoft.Bot.Schema.dll

[cleemullins]

No issues found in Microsoft.Bot.Builder.dll
No issues found in Microsoft.Bot.Builder.AI.Luis.dll
No issues found in Microsoft.Bot.Builder.AI.QnA.dll
No issues found in Microsoft.Bot.Builder.ApplicationInsights.dll
No issues found in Microsoft.Bot.Builder.Azure.dll
No issues found in Microsoft.Bot.Builder.Dialogs.dll
No issues found in Microsoft.Bot.Builder.TemplateManager.dll
No issues found in Microsoft.Bot.Configuration.dll
No issues found in Microsoft.Bot.Connector.dll
No issues found in Microsoft.Bot.Schema.dll

[Bill7zz]

So I'm a bit confused. I see _partitionKey has been added which is great.

... however it's only consumed on "delete".
Read/Write don't seem to use this field at all.

Is this correct and by design for CosmosDB, or is this an oversight in the PR?

Hello @chris, this was an oversight in the PR. The write method doesn't need the partitionKey but the Read method does, so we added new unit tests to validate this and updated the code.

[cleemullins]

@johnataylor - this looks good to me at this point. I've been staring at it a while though, and would appriciate a 2nd set of eyes. Could you take a look?

[cleemullins]

@johnataylor - Could you review. This is the same fix (I think) as the JS fix found here:
microsoft/botbuilder-js#860

feedback incorporated

[sschutten]

I'm trying to understand how this would work. A partition key must be a value within the entity. The guide suggests to use /id for the partition key. This solution assumes the partition key to be the same for the whole bot instance, which in my mind isn't. Because of this, I'm running into this issue and I'm unable to resolve it.