Skip to content

Commit

Permalink
Fixed query responses to return null Content if it is a failure. (#1036)
Browse files Browse the repository at this point in the history 
* Fixed QueryResponse to always return Content as null if it is a failure.

* Updated changelog

* Fixed tests

* Increased timeout on unit test to fix flaky test
  • Loading branch information
@j82w
j82w authored Nov 21, 2019
1 parent 0b7194c commit 3bcab2e
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 24 deletions.
3 changes: 1 addition & 2 deletions Microsoft.Azure.Cosmos/src/Query/v3Query/QueryIterator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public static QueryIterator Create(
count: responseCore.CosmosElements.Count,
responseLengthBytes: responseCore.ResponseLengthBytes,
diagnostics: diagnostics,
serializationOptions: this.cosmosSerializationFormatOptions,
responseHeaders: new CosmosQueryResponseMessageHeaders(
responseCore.ContinuationToken,
responseCore.DisallowContinuationTokenMessage,
Expand Down Expand Up @@ -129,8 +130,6 @@ public static QueryIterator Create(
});
}

queryResponse.CosmosSerializationOptions = this.cosmosSerializationFormatOptions;

return queryResponse;
}

Expand Down
48 changes: 29 additions & 19 deletions Microsoft.Azure.Cosmos/src/Query/v3Query/QueryResponse.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ private QueryResponse(
RequestMessage requestMessage,
CosmosDiagnostics diagnostics,
string errorMessage,
Error error)
Error error,
Lazy<MemoryStream> memoryStream,
CosmosSerializationFormatOptions serializationOptions)
: base(
statusCode: statusCode,
requestMessage: requestMessage,
Expand All @@ -47,11 +49,8 @@ private QueryResponse(
this.CosmosElements = result;
this.Count = count;
this.ResponseLengthBytes = responseLengthBytes;
this.memoryStream = new Lazy<MemoryStream>(() => CosmosElementSerializer.ToStream(
this.QueryHeaders.ContainerRid,
this.CosmosElements,
this.QueryHeaders.ResourceType,
this.CosmosSerializationOptions));
this.memoryStream = memoryStream;
this.CosmosSerializationOptions = serializationOptions;
}

public int Count { get; }
Expand All @@ -60,7 +59,7 @@ public override Stream Content
{
get
{
return this.memoryStream.Value;
return this.memoryStream?.Value;
}
}

Expand All @@ -76,7 +75,7 @@ public override Stream Content
/// </remarks>
internal long ResponseLengthBytes { get; }

internal virtual CosmosSerializationFormatOptions CosmosSerializationOptions { get; set; }
internal virtual CosmosSerializationFormatOptions CosmosSerializationOptions { get; }

internal bool GetHasMoreResults()
{
Expand All @@ -88,7 +87,8 @@ internal static QueryResponse CreateSuccess(
int count,
long responseLengthBytes,
CosmosQueryResponseMessageHeaders responseHeaders,
CosmosDiagnostics diagnostics)
CosmosDiagnostics diagnostics,
CosmosSerializationFormatOptions serializationOptions)
{
if (count < 0)
{
Expand All @@ -100,6 +100,12 @@ internal static QueryResponse CreateSuccess(
throw new ArgumentOutOfRangeException("responseLengthBytes must be positive");
}

Lazy<MemoryStream> memoryStream = new Lazy<MemoryStream>(() => CosmosElementSerializer.ToStream(
responseHeaders.ContainerRid,
result,
responseHeaders.ResourceType,
serializationOptions));

QueryResponse cosmosQueryResponse = new QueryResponse(
result: result,
count: count,
Expand All @@ -109,7 +115,9 @@ internal static QueryResponse CreateSuccess(
statusCode: HttpStatusCode.OK,
errorMessage: null,
error: null,
requestMessage: null);
requestMessage: null,
memoryStream: memoryStream,
serializationOptions: serializationOptions);

return cosmosQueryResponse;
}
Expand All @@ -123,15 +131,17 @@ internal static QueryResponse CreateFailure(
CosmosDiagnostics diagnostics)
{
QueryResponse cosmosQueryResponse = new QueryResponse(
result: Enumerable.Empty<CosmosElement>(),
count: 0,
responseLengthBytes: 0,
responseHeaders: responseHeaders,
diagnostics: diagnostics,
statusCode: statusCode,
errorMessage: errorMessage,
error: error,
requestMessage: requestMessage);
result: Enumerable.Empty<CosmosElement>(),
count: 0,
responseLengthBytes: 0,
responseHeaders: responseHeaders,
diagnostics: diagnostics,
statusCode: statusCode,
errorMessage: errorMessage,
error: error,
requestMessage: requestMessage,
memoryStream: null,
serializationOptions: null);

return cosmosQueryResponse;
}
Expand Down
10 changes: 10 additions & 0 deletions Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosItemTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ public async Task CreateDropItemTest()
Assert.IsNotNull(deleteResponse);
}

[TestMethod]
public async Task NegativeCreateDropItemTest()
{
ToDoActivity testItem = ToDoActivity.CreateRandomToDoActivity();
ResponseMessage response = await this.Container.CreateItemStreamAsync(streamPayload: TestCommon.Serializer.ToStream(testItem), new Cosmos.PartitionKey("BadKey"));
Assert.IsNotNull(response);
Assert.IsNull(response.Content);
Assert.AreEqual(HttpStatusCode.BadRequest, response.StatusCode);
}

[TestMethod]
public async Task CustomSerilizerTest()
{
Expand Down
2 changes: 1 addition & 1 deletion Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/ComparableTaskSchedulerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public async Task SimpleTestAsync()
}
}

bool completionStatus = Task.WaitAll(tasks.ToArray(), TimeSpan.FromSeconds(5));
bool completionStatus = Task.WaitAll(tasks.ToArray(), TimeSpan.FromSeconds(10));
Assert.IsTrue(completionStatus);

foreach (Task task in tasks)
Expand Down
41 changes: 39 additions & 2 deletions Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosQueryUnitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,40 @@ namespace Microsoft.Azure.Cosmos.Tests
[TestClass]
public class CosmosQueryUnitTests
{
[TestMethod]
public void VerifyNegativeCosmosQueryResponseStream()
{
string contianerRid = "mockContainerRid";
string errorMessage = "TestErrorMessage";
string activityId = "TestActivityId";
double requestCharge = 42.42;

Mock<CosmosDiagnostics> mockDiagnostics = new Mock<CosmosDiagnostics>();
CosmosDiagnostics diagnostics = mockDiagnostics.Object;
QueryResponse queryResponse = QueryResponse.CreateFailure(
statusCode: HttpStatusCode.NotFound,
errorMessage: errorMessage,
requestMessage: null,
error: null,
responseHeaders: new CosmosQueryResponseMessageHeaders(
null,
null,
ResourceType.Document,
contianerRid)
{
RequestCharge = requestCharge,
ActivityId = activityId
},
diagnostics: diagnostics);

Assert.AreEqual(HttpStatusCode.NotFound, queryResponse.StatusCode);
Assert.AreEqual(errorMessage, queryResponse.ErrorMessage);
Assert.AreEqual(requestCharge, queryResponse.Headers.RequestCharge);
Assert.AreEqual(activityId, queryResponse.Headers.ActivityId);
Assert.AreEqual(diagnostics, queryResponse.Diagnostics);
Assert.IsNull(queryResponse.Content);
}

[TestMethod]
public void VerifyCosmosQueryResponseStream()
{
Expand All @@ -39,6 +73,7 @@ public void VerifyCosmosQueryResponseStream()
result: responseCore.CosmosElements,
count: responseCore.CosmosElements.Count,
responseLengthBytes: responseCore.ResponseLengthBytes,
serializationOptions: null,
responseHeaders: new CosmosQueryResponseMessageHeaders(
responseCore.ContinuationToken,
responseCore.DisallowContinuationTokenMessage,
Expand All @@ -52,7 +87,7 @@ public void VerifyCosmosQueryResponseStream()

using (Stream stream = queryResponse.Content)
{
using(Stream innerStream = queryResponse.Content)
using (Stream innerStream = queryResponse.Content)
{
Assert.IsTrue(object.ReferenceEquals(stream, innerStream), "Content should return the same stream");
}
Expand All @@ -76,6 +111,7 @@ public void VerifyItemQueryResponseResult()
result: cosmosElements,
count: cosmosElements.Count,
responseLengthBytes: responseCore.ResponseLengthBytes,
serializationOptions: null,
responseHeaders: new CosmosQueryResponseMessageHeaders(
responseCore.ContinuationToken,
responseCore.DisallowContinuationTokenMessage,
Expand Down Expand Up @@ -116,6 +152,7 @@ public void VerifyItemQueryResponseCosmosElements()
result: cosmosElements,
count: cosmosElements.Count,
responseLengthBytes: responseCore.ResponseLengthBytes,
serializationOptions: null,
responseHeaders: new CosmosQueryResponseMessageHeaders(
responseCore.ContinuationToken,
responseCore.DisallowContinuationTokenMessage,
Expand Down Expand Up @@ -323,7 +360,7 @@ public async Task TestCosmosQueryPartitionKeyDefinition()

Mock<IDocumentQueryExecutionComponent> baseContext = new Mock<IDocumentQueryExecutionComponent>();
baseContext.Setup(x => x.DrainAsync(It.IsAny<int>(), It.IsAny<CancellationToken>())).Returns(Task.FromResult<QueryResponseCore>(failure));
Func<string, Task<TryCatch<IDocumentQueryExecutionComponent>>> callBack = x => Task.FromResult<TryCatch<IDocumentQueryExecutionComponent>>(TryCatch<IDocumentQueryExecutionComponent> .FromResult(baseContext.Object));
Func<string, Task<TryCatch<IDocumentQueryExecutionComponent>>> callBack = x => Task.FromResult<TryCatch<IDocumentQueryExecutionComponent>>(TryCatch<IDocumentQueryExecutionComponent>.FromResult(baseContext.Object));
return (callBack, failure);
}
}
Expand Down
2 changes: 2 additions & 0 deletions Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Scenarios/GremlinScenarioTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,7 @@ internal void GetCosmosElementsFromQueryResponseTest(JsonSerializationFormat jso
vertexArray,
count: 2,
responseLengthBytes: vertex1JsonWriterResult.Length + vertex2JsonWriterResult.Length,
serializationOptions: null,
responseHeaders: CosmosQueryResponseMessageHeaders.ConvertToQueryHeaders(
sourceHeaders: null,
resourceType: ResourceType.Document,
Expand Down Expand Up @@ -723,6 +724,7 @@ internal void GetDeserializedObjectsFromQueryResponseTest(JsonSerializationForma
vertexArray,
count: 2,
responseLengthBytes: vertex1JsonWriterResult.Length + vertex2JsonWriterResult.Length,
serializationOptions: null,
responseHeaders: CosmosQueryResponseMessageHeaders.ConvertToQueryHeaders(
sourceHeaders: null,
resourceType: ResourceType.Document,
Expand Down
1 change: 1 addition & 0 deletions changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [#1013](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/1013) Gateway OperationCanceledException are now returned as request timeouts
- [#1020](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/1020) Direct package update removes debug statements
- [#1023](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/1023) Fixed ThroughputResponse.IsReplacePending header mapping
- [#1036](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/1036) Fixed query responses to return null Content if it is a failure

## <a name="3.4.1"/> [3.4.1](https://www.nuget.org/packages/Microsoft.Azure.Cosmos/3.4.1) - 2019-11-06

Expand Down

0 comments on commit 3bcab2e

Please sign in to comment.