Skip to content

Commit

Permalink
Consolidating logic in one place and checking in contract changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@asketagarwal
asketagarwal committed Jan 26, 2021
1 parent 5e1839a commit d088d43
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 141 deletions.
30 changes: 10 additions & 20 deletions Microsoft.Azure.Cosmos/src/Batch/BatchCore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ public override TransactionalBatch CreateItem<T>(
operationIndex: this.operations.Count,
resource: item,
requestOptions: requestOptions,
containerCore: this.container,
cosmosClientContext: this.container.ClientContext));
containerCore: this.container));

return this;
}
Expand All @@ -68,8 +67,7 @@ public override TransactionalBatch CreateItemStream(
operationIndex: this.operations.Count,
resourceStream: streamPayload,
requestOptions: requestOptions,
containerCore: this.container,
cosmosClientContext: this.container.ClientContext));
containerCore: this.container));

return this;
}
Expand All @@ -88,8 +86,7 @@ public override TransactionalBatch ReadItem(
operationIndex: this.operations.Count,
id: id,
requestOptions: requestOptions,
containerCore: this.container,
cosmosClientContext: this.container.ClientContext));
containerCore: this.container));

return this;
}
Expand All @@ -108,8 +105,7 @@ public override TransactionalBatch UpsertItem<T>(
operationIndex: this.operations.Count,
resource: item,
requestOptions: requestOptions,
containerCore: this.container,
cosmosClientContext: this.container.ClientContext));
containerCore: this.container));

return this;
}
Expand All @@ -128,8 +124,7 @@ public override TransactionalBatch UpsertItemStream(
operationIndex: this.operations.Count,
resourceStream: streamPayload,
requestOptions: requestOptions,
containerCore: this.container,
cosmosClientContext: this.container.ClientContext));
containerCore: this.container));

return this;
}
Expand All @@ -155,8 +150,7 @@ public override TransactionalBatch ReplaceItem<T>(
id: id,
resource: item,
requestOptions: requestOptions,
containerCore: this.container,
cosmosClientContext: this.container.ClientContext));
containerCore: this.container));

return this;
}
Expand All @@ -182,8 +176,7 @@ public override TransactionalBatch ReplaceItemStream(
id: id,
resourceStream: streamPayload,
requestOptions: requestOptions,
containerCore: this.container,
cosmosClientContext: this.container.ClientContext));
containerCore: this.container));

return this;
}
Expand All @@ -202,8 +195,7 @@ public override TransactionalBatch DeleteItem(
operationIndex: this.operations.Count,
id: id,
requestOptions: requestOptions,
containerCore: this.container,
cosmosClientContext: this.container.ClientContext));
containerCore: this.container));

return this;
}
Expand Down Expand Up @@ -261,8 +253,7 @@ public virtual TransactionalBatch PatchItemStream(
id: id,
resourceStream: patchStream,
requestOptions: requestOptions,
containerCore: this.container,
cosmosClientContext: this.container.ClientContext));
containerCore: this.container));

return this;
}
Expand Down Expand Up @@ -301,8 +292,7 @@ TransactionalBatch PatchItem(
id: id,
resource: patchOperations,
requestOptions: requestOptions,
containerCore: this.container,
cosmosClientContext: this.container.ClientContext));
containerCore: this.container));

return this;
}
Expand Down
30 changes: 8 additions & 22 deletions Microsoft.Azure.Cosmos/src/Batch/ItemBatchOperation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ public ItemBatchOperation(
ContainerInternal containerCore,
string id = null,
Stream resourceStream = null,
TransactionalBatchItemRequestOptions requestOptions = null,
CosmosClientContext cosmosClientContext = null)
TransactionalBatchItemRequestOptions requestOptions = null)
{
this.OperationType = operationType;
this.OperationIndex = operationIndex;
Expand All @@ -63,7 +62,7 @@ public ItemBatchOperation(
this.ResourceStream = resourceStream;
this.RequestOptions = requestOptions;
this.DiagnosticsContext = null;
this.ClientContext = cosmosClientContext;
this.ClientContext = containerCore.ClientContext;
}

public PartitionKey? PartitionKey { get; internal set; }
Expand Down Expand Up @@ -174,18 +173,6 @@ internal static Result WriteOperation(ref RowWriter writer, TypeArgument typeArg
return r;
}
}

if (ItemRequestOptions.ShouldSetNoContentHeader(
options.EnableContentResponseOnWrite,
options.EnableContentResponseOnRead,
operation.OperationType))
{
r = writer.WriteBool("minimalReturnPreference", true);
if (r != Result.Success)
{
return r;
}
}

if (options.IfMatchEtag != null)
{
Expand Down Expand Up @@ -258,10 +245,10 @@ internal static Result WriteOperation(ref RowWriter writer, TypeArgument typeArg
}
}

// If EnableContentResponse is set at Client level and not at Indivisual Item Level
if (operation.ClientContext != null
&& RequestInvokerHandler.ShouldSetClientLevelNoContentResponseHeaders(operation.RequestOptions,
operation.ClientContext.ClientOptions, operation.OperationType, ResourceType.Document))
if (RequestInvokerHandler.ShouldSetNoContentResponseHeaders(operation.RequestOptions,
operation.ClientContext?.ClientOptions,
operation.OperationType,
ResourceType.Document))
{
r = writer.WriteBool("minimalReturnPreference", true);
if (r != Result.Success)
Expand Down Expand Up @@ -402,9 +389,8 @@ public ItemBatchOperation(
T resource,
ContainerInternal containerCore,
string id = null,
TransactionalBatchItemRequestOptions requestOptions = null,
CosmosClientContext cosmosClientContext = null)
: base(operationType, operationIndex, containerCore: containerCore, id: id, requestOptions: requestOptions, cosmosClientContext: cosmosClientContext)
TransactionalBatchItemRequestOptions requestOptions = null)
: base(operationType, operationIndex, containerCore: containerCore, id: id, requestOptions: requestOptions)
{
this.Resource = resource;
}
Expand Down
12 changes: 0 additions & 12 deletions Microsoft.Azure.Cosmos/src/Batch/TransactionalBatchItemRequestOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,6 @@ public class TransactionalBatchItemRequestOptions : RequestOptions
/// </remarks>
public bool? EnableContentResponseOnWrite { get; set; }

/// <summary>
/// Gets or sets the boolean to only return the headers and status code in
/// the Cosmos DB response for read item operations like ReadItem
/// This removes the resource from the response. This reduces networking and CPU load by not sending
/// the resource back over the network and serializing it on the client.
/// </summary>
/// <remarks>
/// This is optimal for workloads where the returned resource is not used.
/// </remarks>
internal bool? EnableContentResponseOnRead { get; set; }

internal static TransactionalBatchItemRequestOptions FromItemRequestOptions(ItemRequestOptions itemRequestOptions)
{
if (itemRequestOptions == null)
Expand All @@ -55,7 +44,6 @@ internal static TransactionalBatchItemRequestOptions FromItemRequestOptions(Item
IfNoneMatchEtag = itemRequestOptions.IfNoneMatchEtag,
Properties = itemRequestOptions.Properties,
EnableContentResponseOnWrite = itemRequestOptions.EnableContentResponseOnWrite,
EnableContentResponseOnRead = itemRequestOptions.EnableContentResponseOnRead,
IsEffectivePartitionKeyRouting = itemRequestOptions.IsEffectivePartitionKeyRouting
};
return batchItemRequestOptions;
Expand Down
47 changes: 38 additions & 9 deletions Microsoft.Azure.Cosmos/src/Handler/RequestInvokerHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ public override async Task<ResponseMessage> SendAsync(
}

// Adds the NoContent header if not already added based on Client Level flag
if (RequestInvokerHandler.ShouldSetClientLevelNoContentResponseHeaders(request.RequestOptions, this.client.ClientOptions,
request.OperationType, request.ResourceType))
if (RequestInvokerHandler.ShouldSetNoContentResponseHeaders(request.RequestOptions, this.client.ClientOptions, request.OperationType, request.ResourceType))
{
request.Headers.Add(HttpConstants.HttpHeaders.Prefer, HttpConstants.HttpHeaderValues.PreferReturnMinimal);
}
Expand Down Expand Up @@ -389,7 +388,7 @@ private async Task ValidateAndSetConsistencyLevelAsync(RequestMessage requestMes
}
}

internal static bool ShouldSetClientLevelNoContentResponseHeaders(RequestOptions requestOptions,
internal static bool ShouldSetNoContentResponseHeaders(RequestOptions requestOptions,
CosmosClientOptions clientOptions,
OperationType operationType,
ResourceType resourceType)
Expand All @@ -399,22 +398,52 @@ internal static bool ShouldSetClientLevelNoContentResponseHeaders(RequestOptions
return false;
}

if (requestOptions == null
|| ((requestOptions is ItemRequestOptions itemRequestOptions) && (!itemRequestOptions.EnableContentResponseOnWrite.HasValue))
|| ((requestOptions is TransactionalBatchItemRequestOptions batchRequestOptions) && (!batchRequestOptions.EnableContentResponseOnWrite.HasValue)))
if (requestOptions == null)
{
return RequestInvokerHandler.IsClientNoResponseSet(clientOptions, operationType);
}

if (requestOptions is ItemRequestOptions itemRequestOptions)
{
if (itemRequestOptions.EnableContentResponseOnWrite.HasValue)
{
return IsItemNoRepsonseSet(itemRequestOptions.EnableContentResponseOnWrite.Value, operationType);
}
else
{
return IsClientNoResponseSet(clientOptions, operationType);
}
}

if (requestOptions is TransactionalBatchItemRequestOptions batchRequestOptions)
{
if (batchRequestOptions.EnableContentResponseOnWrite.HasValue)
{
return IsItemNoRepsonseSet(batchRequestOptions.EnableContentResponseOnWrite.Value, operationType);
}
else
{
return IsClientNoResponseSet(clientOptions, operationType);
}
}

return false;
}

private static bool IsItemNoRepsonseSet(bool enableContentResponseOnWrite, OperationType operationType)
{
return !enableContentResponseOnWrite &&
(operationType == OperationType.Create ||
operationType == OperationType.Replace ||
operationType == OperationType.Upsert ||
operationType == OperationType.Patch);
}

private static bool IsClientNoResponseSet(CosmosClientOptions clientOptions, OperationType operationType)
{
return clientOptions != null
&& ItemRequestOptions.ShouldSetNoContentHeader(enableContentResponseOnWrite: clientOptions.EnableContentResponseOnWrite,
enableContentResponseOnRead: null,
operationType: operationType);
&& clientOptions.EnableContentResponseOnWrite.HasValue
&& RequestInvokerHandler.IsItemNoRepsonseSet(clientOptions.EnableContentResponseOnWrite.Value, operationType);
}
}
}
50 changes: 0 additions & 50 deletions Microsoft.Azure.Cosmos/src/RequestOptions/ItemRequestOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,27 +118,6 @@ public ConsistencyLevel? ConsistencyLevel
/// </remarks>
public bool? EnableContentResponseOnWrite { get; set; }

/// <summary>
/// Gets or sets the boolean to only return the headers and status code in
/// the Cosmos DB response for read item operations like ReadItemAsync
/// This removes the resource from the response. This reduces networking and CPU load by not sending
/// the resource back over the network and serializing it on the client.
/// </summary>
/// <example>
/// <code language="c#">
/// <![CDATA[
/// ItemRequestOption requestOptions = new ItemRequestOptions() { EnableContentResponseOnRead = true };
/// ItemResponse itemResponse = await this.container.ReadItemAsync<ToDoActivity>(tests, new PartitionKey(test.status), requestOptions);
/// Assert.AreEqual(HttpStatusCode.Created, itemResponse.StatusCode);
/// Assert.IsNull(itemResponse.Resource);
/// ]]>
/// </code>
/// </example>
/// <remarks>
/// This is optimal for workloads where the returned resource is not used.
/// </remarks>
internal bool? EnableContentResponseOnRead { get; set; }

/// <summary>
/// Fill the CosmosRequestMessage headers with the set properties
/// </summary>
Expand All @@ -163,36 +142,7 @@ internal override void PopulateRequestOptions(RequestMessage request)
}

RequestOptions.SetSessionToken(request, this.SessionToken);

if (ItemRequestOptions.ShouldSetNoContentHeader(
this.EnableContentResponseOnWrite,
this.EnableContentResponseOnRead,
request.OperationType))
{
request.Headers.Add(HttpConstants.HttpHeaders.Prefer, HttpConstants.HttpHeaderValues.PreferReturnMinimal);
}

base.PopulateRequestOptions(request);
}

internal static bool ShouldSetNoContentHeader(
bool? enableContentResponseOnWrite,
bool? enableContentResponseOnRead,
OperationType operationType)
{
if (enableContentResponseOnRead.HasValue &&
!enableContentResponseOnRead.Value &&
operationType == OperationType.Read)
{
return true;
}

return enableContentResponseOnWrite.HasValue &&
!enableContentResponseOnWrite.Value &&
(operationType == OperationType.Create ||
operationType == OperationType.Replace ||
operationType == OperationType.Upsert ||
operationType == OperationType.Patch);
}
}
}
28 changes: 0 additions & 28 deletions ...oft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosContentResponseTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,20 +61,6 @@ await this.Validate(
this.ValidateItemResponse);
}

[TestMethod]
public async Task ItemReadNoResponseTest()
{
ItemRequestOptions requestOptions = new ItemRequestOptions()
{
EnableContentResponseOnRead = false
};

await this.Validate(
requestOptions,
this.ValidateItemResponse,
this.ValidateItemNoContentResponse);
}

private async Task Validate(
ItemRequestOptions requestOptions,
Action<ItemResponse<ToDoActivity>> ValidateWrite,
Expand Down Expand Up @@ -138,20 +124,6 @@ await this.ValidateItemStream(
this.ValidateItemStreamResponse);
}

[TestMethod]
public async Task ItemStreamReadNoResponseTest()
{
ItemRequestOptions requestOptions = new ItemRequestOptions()
{
EnableContentResponseOnRead = false
};

await this.ValidateItemStream(
requestOptions,
this.ValidateItemStreamResponse,
this.ValidateItemStreamNoContentResponse);
}

private async Task ValidateItemStream(
ItemRequestOptions requestOptions,
Action<ResponseMessage> ValidateWrite,
Expand Down
Loading

0 comments on commit d088d43

Please sign in to comment.