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

Fix a bug causing query response to create a new stream for each content call #901

Merged
merged 10 commits into from
Oct 22, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ internal static CosmosArray ToCosmosElements(
/// <param name="resourceType">The resource type</param>
/// <param name="cosmosSerializationOptions">The custom serialization options. This allows custom serialization types like BSON, JSON, or other formats</param>
/// <returns>Returns a memory stream of cosmos elements. By default the memory stream will contain JSON.</returns>
internal static Stream ToStream(
internal static MemoryStream ToStream(
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you have finally come to the dark side on Stream vs MemoryStream and their disposability :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I honestly would like to convert all streams to new Memory objects.

string containerRid,
IEnumerable<CosmosElement> cosmosElements,
ResourceType resourceType,
Expand Down
19 changes: 14 additions & 5 deletions Microsoft.Azure.Cosmos/src/Query/v3Query/QueryResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ namespace Microsoft.Azure.Cosmos
/// </summary>
internal class QueryResponse : ResponseMessage
{
private readonly Lazy<MemoryStream> memoryStream;

/// <summary>
/// Used for unit testing only
/// </summary>
Expand Down Expand Up @@ -45,15 +47,22 @@ private QueryResponse(
this.Count = count;
this.ResponseLengthBytes = responseLengthBytes;
this.queryMetrics = queryMetrics;
this.memoryStream = new Lazy<MemoryStream>(() => CosmosElementSerializer.ToStream(
this.QueryHeaders.ContainerRid,
this.CosmosElements,
this.QueryHeaders.ResourceType,
this.CosmosSerializationOptions));
}

public int Count { get; }

public override Stream Content => CosmosElementSerializer.ToStream(
this.QueryHeaders.ContainerRid,
this.CosmosElements,
this.QueryHeaders.ResourceType,
this.CosmosSerializationOptions);
public override Stream Content
{
get
{
return this.memoryStream.Value;
}
}

internal virtual IEnumerable<CosmosElement> CosmosElements { get; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,7 @@ public async Task ItemQueryStreamSerializationSetting()
MemoryStream memoryStream = new MemoryStream();
response.Content.CopyTo(memoryStream);
byte[] content = memoryStream.ToArray();
response.Content.Position = 0;

// Examine the first buffer byte to determine the serialization format
byte firstByte = content[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace Microsoft.Azure.Cosmos.Tests
{
using System;
using System.Collections.Generic;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Azure.Cosmos.Query;
Expand All @@ -17,6 +18,42 @@ namespace Microsoft.Azure.Cosmos.Tests
[TestClass]
public class CosmosQueryUnitTests
{
[TestMethod]
public void VerifyCosmosQueryResponseStream()
{
string contianerRid = "mockContainerRid";
(QueryResponseCore response, IList<ToDoItem> items) factoryResponse = QueryResponseMessageFactory.Create(
itemIdPrefix: $"TestPage",
continuationToken: "SomeContinuationToken",
collectionRid: contianerRid,
itemCount: 100);

QueryResponseCore responseCore = factoryResponse.response;

QueryResponse queryResponse = QueryResponse.CreateSuccess(
result: responseCore.CosmosElements,
count: responseCore.CosmosElements.Count,
responseLengthBytes: responseCore.ResponseLengthBytes,
queryMetrics: responseCore.QueryMetrics,
responseHeaders: new CosmosQueryResponseMessageHeaders(
responseCore.ContinuationToken,
responseCore.DisallowContinuationTokenMessage,
ResourceType.Document,
contianerRid)
{
RequestCharge = responseCore.RequestCharge,
ActivityId = responseCore.ActivityId
});

using (Stream stream = queryResponse.Content)
{
using(Stream innerStream = queryResponse.Content)
{
Assert.IsTrue(object.ReferenceEquals(stream, innerStream), "Content should return the same stream");
}
}
}

[TestMethod]
public async Task TestCosmosQueryExecutionComponentOnFailure()
{
Expand Down
7 changes: 5 additions & 2 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,18 @@ Preview features are treated as a separate branch and will not be included in th
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased
## Unreleased

### Fixed

- [#901](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/901) Fix a bug causing query response to create a new stream for each content call

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

### Fixed

- [#905](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/909) Fixed linq camel case bug


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

### Fixed
Expand Down