From 8fdfe688d306f7da14b87a3aea2fd07c8d26f419 Mon Sep 17 00:00:00 2001 From: j82w Date: Fri, 23 Aug 2019 11:37:11 -0700 Subject: [PATCH 1/3] Query iterator HasMoreResults now returns false if an exception is hit (#726) * Fix HasMoreResults to return false if an exception is hit. * Updated changelog --- .../CosmosQueryExecutionContextFactory.cs | 36 +++++++++++--- .../CosmosNotFoundTests.cs | 49 ++++++++++--------- changelog.md | 2 + 3 files changed, 57 insertions(+), 30 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/Query/CosmosQueryExecutionContextFactory.cs b/Microsoft.Azure.Cosmos/src/Query/CosmosQueryExecutionContextFactory.cs index 9cbd1c7119..e460c38206 100644 --- a/Microsoft.Azure.Cosmos/src/Query/CosmosQueryExecutionContextFactory.cs +++ b/Microsoft.Azure.Cosmos/src/Query/CosmosQueryExecutionContextFactory.cs @@ -31,6 +31,11 @@ internal sealed class CosmosQueryExecutionContextFactory : FeedIterator private readonly string InitialUserContinuationToken; private CosmosQueryExecutionContext innerExecutionContext; + /// + /// Store the failed response + /// + private ResponseMessage responseMessageException; + /// /// Test flag for making the query use the opposite code path for query plan retrieval. /// If the SDK would have went to Gateway, then it will use ServiceInterop and visa versa. @@ -108,35 +113,52 @@ public override bool HasMoreResults { get { + // No more results if an exception is hit + if (this.responseMessageException != null) + { + return false; + } + return this.innerExecutionContext != null ? !this.innerExecutionContext.IsDone : true; } } public override async Task ReadNextAsync(CancellationToken cancellationToken) { + if (this.responseMessageException != null) + { + return this.responseMessageException; + } + // This catches exception thrown by the pipeline and converts it to QueryResponse + ResponseMessage response; try { - return await this.ExecuteNextHelperAsync(cancellationToken); + response = await this.ExecuteNextHelperAsync(cancellationToken); } catch (DocumentClientException exception) { - return exception.ToCosmosResponseMessage(request: null); + response = exception.ToCosmosResponseMessage(request: null); } catch (CosmosException exception) { - return exception.ToCosmosResponseMessage(request: null); + response = exception.ToCosmosResponseMessage(request: null); } catch (AggregateException ae) { - ResponseMessage errorMessage = TransportHandler.AggregateExceptionConverter(ae, null); - if (errorMessage != null) + response = TransportHandler.AggregateExceptionConverter(ae, null); + if (response == null) { - return errorMessage; + throw; } + } - throw; + if (!response.IsSuccessStatusCode) + { + this.responseMessageException = response; } + + return response; } /// diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosNotFoundTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosNotFoundTests.cs index 8b51d4e2da..2b1ae5bf1a 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosNotFoundTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosNotFoundTests.cs @@ -7,17 +7,16 @@ namespace Microsoft.Azure.Cosmos.SDK.EmulatorTests using System; using System.IO; using System.Net; - using System.Threading; using System.Threading.Tasks; using Microsoft.VisualStudio.TestTools.UnitTesting; - + using Moq; [TestClass] public class CosmosNotFoundTests { public const string DoesNotExist = "DoesNotExist-69E1BD04-EC99-449B-9365-34DA9F4D4ECE"; private static CosmosClient client = null; - + [ClassInitialize] public static void Initialize(TestContext textContext) { @@ -53,29 +52,27 @@ public async Task ValidateQueryNotFoundResponse() await container.DeleteContainerAsync(); - var crossPartitionQueryIterator = container.GetItemQueryStreamIterator( - "select * from t where true", - requestOptions: new QueryRequestOptions() { MaxConcurrency= 2}); + FeedIterator crossPartitionQueryIterator = container.GetItemQueryStreamIterator( + "select * from t where true", + requestOptions: new QueryRequestOptions() { MaxConcurrency = 2 }); - var queryResponse = await crossPartitionQueryIterator.ReadNextAsync(); - Assert.IsNotNull(queryResponse); - Assert.AreEqual(HttpStatusCode.NotFound, queryResponse.StatusCode); + await this.VerifyQueryNotFoundResponse(crossPartitionQueryIterator); - var queryIterator = container.GetItemQueryStreamIterator( + FeedIterator queryIterator = container.GetItemQueryStreamIterator( "select * from t where true", requestOptions: new QueryRequestOptions() - { - MaxConcurrency = 1, - PartitionKey = new Cosmos.PartitionKey("testpk"), + { + MaxConcurrency = 1, + PartitionKey = new Cosmos.PartitionKey("testpk"), }); - this.VerifyNotFoundResponse(await queryIterator.ReadNextAsync()); + await this.VerifyQueryNotFoundResponse(queryIterator); - var crossPartitionQueryIterator2 = container.GetItemQueryStreamIterator( - "select * from t where true", + FeedIterator crossPartitionQueryIterator2 = container.GetItemQueryStreamIterator( + "select * from t where true", requestOptions: new QueryRequestOptions() { MaxConcurrency = 2 }); - this.VerifyQueryNotFoundResponse(await crossPartitionQueryIterator2.ReadNextAsync()); + await this.VerifyQueryNotFoundResponse(crossPartitionQueryIterator2); await db.DeleteAsync(); } @@ -118,13 +115,13 @@ private async Task ItemOperations(Container container, bool containerNotExist) Stream create = TestCommon.Serializer.ToStream(randomItem); this.VerifyNotFoundResponse(await container.CreateItemStreamAsync(create, new PartitionKey(randomItem.pk))); - var queryIterator = container.GetItemQueryStreamIterator( - "select * from t where true", + FeedIterator queryIterator = container.GetItemQueryStreamIterator( + "select * from t where true", requestOptions: new QueryRequestOptions() { MaxConcurrency = 2 }); this.VerifyNotFoundResponse(await queryIterator.ReadNextAsync()); - var feedIterator = container.GetItemQueryStreamIterator(); + FeedIterator feedIterator = container.GetItemQueryStreamIterator(); this.VerifyNotFoundResponse(await feedIterator.ReadNextAsync()); dynamic randomUpsertItem = new { id = DoesNotExist, pk = DoesNotExist, status = 42 }; @@ -145,10 +142,16 @@ private async Task ItemOperations(Container container, bool containerNotExist) streamPayload: replace)); } - private void VerifyQueryNotFoundResponse(ResponseMessage response) + private async Task VerifyQueryNotFoundResponse(FeedIterator iterator) { - Assert.IsNotNull(response); - Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode); + // Verify that even if the user ignores the HasMoreResults it still returns the exception + for(int i = 0; i < 3; i++) + { + ResponseMessage response = await iterator.ReadNextAsync(); + Assert.IsNotNull(response); + Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode); + Assert.IsFalse(iterator.HasMoreResults); + } } private void VerifyNotFoundResponse(ResponseMessage response) diff --git a/changelog.md b/changelog.md index 0ef6815f32..a31be6b319 100644 --- a/changelog.md +++ b/changelog.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- [#726](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/726) Query iterator HasMoreResults now returns false if an exception is hit + ## [3.1.1](https://www.nuget.org/packages/Microsoft.Azure.Cosmos/3.1.1) - 2019-08-12 From d5f4a0aa41ec6602507833ff2bb2796e41e2f87f Mon Sep 17 00:00:00 2001 From: j82w Date: Mon, 26 Aug 2019 07:18:29 -0700 Subject: [PATCH 2/3] Fixed changelog (#734) --- changelog.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/changelog.md b/changelog.md index a31be6b319..7739173610 100644 --- a/changelog.md +++ b/changelog.md @@ -8,13 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - [#100](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/100) Configurable Tcp settings to CosmosClientOptions +- [#622](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/622) Added CRUD and query operations for Users and Permissions which enables [ResourceToken](https://docs.microsoft.com/en-us/azure/cosmos-db/secure-access-to-data#resource-tokens) support ### Fixed - [#726](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/726) Query iterator HasMoreResults now returns false if an exception is hit - - ## [3.1.1](https://www.nuget.org/packages/Microsoft.Azure.Cosmos/3.1.1) - 2019-08-12 ### Added @@ -33,7 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## 3.1.0 - 2019-07-29 - Unlisted ### Added -- [#622](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/622) Added CRUD and query operations for Users and Permissions which enables [ResourceToken](https://docs.microsoft.com/en-us/azure/cosmos-db/secure-access-to-data#resource-tokens) support + - [#541](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/541) Added consistency level to client and query options - [#544](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/544) Added continuation token support for LINQ - [#557](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/557) Added trigger options to item request options From d4befb6c4911a460e26a9039b76fa48458726433 Mon Sep 17 00:00:00 2001 From: Matias Quaranta Date: Mon, 26 Aug 2019 10:05:42 -0700 Subject: [PATCH 3/3] Adding tests to cover Batch request overflow (#732) * Adding new tests * Offset --- .../src/Batch/ServerBatchRequest.cs | 3 +- ...artitionKeyRangeServerBatchRequestTests.cs | 92 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/Microsoft.Azure.Cosmos/src/Batch/ServerBatchRequest.cs b/Microsoft.Azure.Cosmos/src/Batch/ServerBatchRequest.cs index dd4e8818d1..e47e9d6246 100644 --- a/Microsoft.Azure.Cosmos/src/Batch/ServerBatchRequest.cs +++ b/Microsoft.Azure.Cosmos/src/Batch/ServerBatchRequest.cs @@ -134,7 +134,8 @@ protected async Task> CreateBodyStreamAsync( throw new RequestEntityTooLargeException(RMResources.RequestTooLarge); } - return new ArraySegment(operations.Array, materializedCount, operations.Count - materializedCount); + int overflowOperations = operations.Count - this.operations.Count; + return new ArraySegment(operations.Array, this.operations.Count + operations.Offset, overflowOperations); } private Result WriteOperation(long index, out ReadOnlyMemory buffer) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/PartitionKeyRangeServerBatchRequestTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/PartitionKeyRangeServerBatchRequestTests.cs index 84ced2be2e..cffe8d072e 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/PartitionKeyRangeServerBatchRequestTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/PartitionKeyRangeServerBatchRequestTests.cs @@ -59,5 +59,97 @@ public async Task OverflowsBasedOnCount() Assert.AreEqual(operations[1].Id, pendingOperations[0].Id); Assert.AreEqual(operations[2].Id, pendingOperations[1].Id); } + + /// + /// Verifies that the pending operations algorithm takes into account Offset + /// + /// + [TestMethod] + public async Task OverflowsBasedOnCount_WithOffset() + { + List operations = new List() + { + CreateItemBatchOperation("1"), + CreateItemBatchOperation("2"), + CreateItemBatchOperation("3") + }; + + // Setting max count to 1 + (PartitionKeyRangeServerBatchRequest request, ArraySegment pendingOperations) = await PartitionKeyRangeServerBatchRequest.CreateAsync("0", new ArraySegment(operations.ToArray(), 1, 2), 200000, 1, false, new CosmosJsonDotNetSerializer(), default(CancellationToken)); + + Assert.AreEqual(1, request.Operations.Count); + // The first element is not taken into account due to an Offset of 1 + Assert.AreEqual(operations[1].Id, request.Operations[0].Id); + Assert.AreEqual(1, pendingOperations.Count); + Assert.AreEqual(operations[2].Id, pendingOperations[0].Id); + } + + [TestMethod] + public async Task PartitionKeyRangeServerBatchRequestSizeTests() + { + const int docSizeInBytes = 250; + const int operationCount = 10; + + foreach (int expectedOperationCount in new int[] { 1, 2, 5, 10 }) + { + await PartitionKeyRangeServerBatchRequestTests.VerifyServerRequestCreationsBySizeAsync(expectedOperationCount, operationCount, docSizeInBytes); + await PartitionKeyRangeServerBatchRequestTests.VerifyServerRequestCreationsByCountAsync(expectedOperationCount, operationCount, docSizeInBytes); + } + } + + private static async Task VerifyServerRequestCreationsBySizeAsync( + int expectedOperationCount, + int operationCount, + int docSizeInBytes) + { + const int perRequestOverheadEstimateInBytes = 30; + const int perDocOverheadEstimateInBytes = 50; + int maxServerRequestBodyLength = ((docSizeInBytes + perDocOverheadEstimateInBytes) * expectedOperationCount) + perRequestOverheadEstimateInBytes; + int maxServerRequestOperationCount = int.MaxValue; + + (PartitionKeyRangeServerBatchRequest request, ArraySegment overflow) = await PartitionKeyRangeServerBatchRequestTests.GetBatchWithCreateOperationsAsync(operationCount, maxServerRequestBodyLength, maxServerRequestOperationCount, docSizeInBytes); + + Assert.AreEqual(expectedOperationCount, request.Operations.Count); + Assert.AreEqual(overflow.Count, operationCount - request.Operations.Count); + } + + private static async Task VerifyServerRequestCreationsByCountAsync( + int expectedOperationCount, + int operationCount, + int docSizeInBytes) + { + int maxServerRequestBodyLength = int.MaxValue; + int maxServerRequestOperationCount = expectedOperationCount; + + (PartitionKeyRangeServerBatchRequest request, ArraySegment overflow) = await PartitionKeyRangeServerBatchRequestTests.GetBatchWithCreateOperationsAsync(operationCount, maxServerRequestBodyLength, maxServerRequestOperationCount, docSizeInBytes); + + Assert.AreEqual(expectedOperationCount, request.Operations.Count); + Assert.AreEqual(overflow.Count, operationCount - request.Operations.Count); + } + + private static async Task>> GetBatchWithCreateOperationsAsync( + int operationCount, + int maxServerRequestBodyLength, + int maxServerRequestOperationCount, + int docSizeInBytes = 20) + { + List operations = new List(); + + byte[] body = new byte[docSizeInBytes]; + Random random = new Random(); + random.NextBytes(body); + for (int i = 0; i < operationCount; i++) + { + operations.Add(new ItemBatchOperation(OperationType.Create, 0, string.Empty, new MemoryStream(body))); + } + + return await PartitionKeyRangeServerBatchRequest.CreateAsync("0", + new ArraySegment(operations.ToArray()), + maxServerRequestBodyLength, + maxServerRequestOperationCount, + false, + new CosmosJsonDotNetSerializer(), + default(CancellationToken)); + } } }