From d8de6484dd1c0f2c64b8e4892f19f1a9eb7dcb45 Mon Sep 17 00:00:00 2001 From: Meghana-Palaparthi Date: Wed, 20 May 2026 18:13:03 -0500 Subject: [PATCH 1/3] [Internal] DTS: Removes dead code and refactors to manual JSON deserialization - Remove unused copy constructor, ErrorMessage property, ITrace plumbing, and per-operation ActivityId/Trace dead stores from response classes - Replace System.Text.Json attribute-based deserialization with manual case-insensitive JsonElement.TryGetProperty parsing - Make SessionToken, PartitionKeyRangeId, and SubStatusCode internal (were public only to satisfy STJ reflection requirements) - Remove public SubStatusCodeValue bridge property (no longer needed) - Add null-safety guards for string fields in JSON deserialization --- .../DistributedTransactionOperationResult.cs | 133 ++++++++---------- .../DistributedTransactionResponse.cs | 34 +---- .../DistributedTransactionResponseTests.cs | 6 +- 3 files changed, 62 insertions(+), 111 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs index 0f1efa364f..e7743dbac7 100644 --- a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs +++ b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs @@ -8,9 +8,7 @@ namespace Microsoft.Azure.Cosmos using System.IO; using System.Net; using System.Text.Json; - using System.Text.Json.Serialization; using Microsoft.Azure.Cosmos.Core.Trace; - using Microsoft.Azure.Cosmos.Tracing; using Microsoft.Azure.Documents; /// @@ -28,117 +26,47 @@ internal DistributedTransactionOperationResult(HttpStatusCode statusCode) this.StatusCode = statusCode; } - internal DistributedTransactionOperationResult(DistributedTransactionOperationResult other) - { - this.Index = other.Index; - this.StatusCode = other.StatusCode; - this.SubStatusCode = other.SubStatusCode; - this.ETag = other.ETag; - this.ResourceStream = other.ResourceStream; - this.SessionToken = other.SessionToken; - this.PartitionKeyRangeId = other.PartitionKeyRangeId; - this.RequestCharge = other.RequestCharge; - this.ActivityId = other.ActivityId; - this.Trace = other.Trace; - } - - /// - /// Initializes a new instance of the class. - /// - /// - /// Must be public for System.Text.Json reflection-based deserialization. - /// System.Text.Json 6.x only scans BindingFlags.Public constructors when resolving - /// ; non-public constructors are not found. - /// Support for non-public constructors was added in System.Text.Json 7.0. - /// - [JsonConstructor] - public DistributedTransactionOperationResult() + internal DistributedTransactionOperationResult() { } /// /// Gets the index of this operation within the distributed transaction. /// - [JsonInclude] - [JsonPropertyName("index")] public virtual int Index { get; internal set; } /// /// Gets the HTTP status code returned by the operation. /// - [JsonInclude] - [JsonPropertyName("statusCode")] public virtual HttpStatusCode StatusCode { get; internal set; } /// /// Gets a value indicating whether the HTTP status code returned by the operation indicates success. /// - [JsonIgnore] public virtual bool IsSuccessStatusCode => (int)this.StatusCode >= 200 && (int)this.StatusCode <= 299; /// /// Gets the entity tag (ETag) associated with the operation result. /// The ETag is used for concurrency control and represents the version of the resource. /// - [JsonInclude] - [JsonPropertyName("etag")] public virtual string ETag { get; internal set; } - /// - /// Gets the session token associated with the operation result. - /// - [JsonInclude] - [JsonPropertyName("sessionToken")] - public virtual string SessionToken { get; internal set; } - - /// - /// Gets the raw partition key range ID emitted by the server. - /// - [JsonInclude] - [JsonPropertyName("partitionKeyRangeId")] - public virtual string PartitionKeyRangeId { get; internal set; } - /// /// Gets the resource stream associated with the operation result. /// The stream contains the raw response payload returned by the operation. /// - [JsonIgnore] public virtual Stream ResourceStream { get; internal set; } /// /// Request charge in request units for the operation. /// - [JsonInclude] - [JsonPropertyName("requestCharge")] public virtual double RequestCharge { get; internal set; } - [JsonIgnore] internal virtual SubStatusCodes SubStatusCode { get; set; } - /// - /// Gets the sub-status code value as an unsigned integer. - /// - [JsonInclude] - [JsonPropertyName("subStatusCode")] - public virtual uint SubStatusCodeValue - { - get => (uint)this.SubStatusCode; - internal set => this.SubStatusCode = (SubStatusCodes)value; - } - - /// - /// ActivityId related to the operation. - /// - [JsonIgnore] - internal virtual string ActivityId { get; set; } - - [JsonIgnore] - internal ITrace Trace { get; set; } + internal virtual string SessionToken { get; set; } - private static readonly JsonSerializerOptions CaseInsensitiveOptions = new JsonSerializerOptions - { - PropertyNameCaseInsensitive = true, - }; + internal virtual string PartitionKeyRangeId { get; set; } /// /// Creates a from a JSON element. @@ -147,10 +75,44 @@ public virtual uint SubStatusCodeValue /// The deserialized operation result with a canonical session token. internal static DistributedTransactionOperationResult FromJson(JsonElement json) { - DistributedTransactionOperationResult result = JsonSerializer.Deserialize(json, DistributedTransactionOperationResult.CaseInsensitiveOptions) - ?? throw new JsonException($"Failed to deserialize DTC operation result: Deserialize returned null. JSON element kind: '{json.ValueKind}'."); + DistributedTransactionOperationResult result = new DistributedTransactionOperationResult(); + + if (TryGetProperty(json, "index", out JsonElement indexEl) && indexEl.TryGetInt32(out int index)) + { + result.Index = index; + } - if (json.TryGetProperty(DistributedTransactionSerializer.ResourceBody, out JsonElement resourceBody) + if (TryGetProperty(json, "statusCode", out JsonElement statusCodeEl) && statusCodeEl.TryGetInt32(out int statusCode)) + { + result.StatusCode = (HttpStatusCode)statusCode; + } + + if (TryGetProperty(json, "subStatusCode", out JsonElement subStatusEl) && subStatusEl.TryGetUInt32(out uint subStatus)) + { + result.SubStatusCode = (SubStatusCodes)subStatus; + } + + if (TryGetProperty(json, "etag", out JsonElement etagEl) && etagEl.ValueKind == JsonValueKind.String) + { + result.ETag = etagEl.GetString(); + } + + if (TryGetProperty(json, "sessionToken", out JsonElement sessionTokenEl) && sessionTokenEl.ValueKind == JsonValueKind.String) + { + result.SessionToken = sessionTokenEl.GetString(); + } + + if (TryGetProperty(json, "partitionKeyRangeId", out JsonElement pkRangeIdEl) && pkRangeIdEl.ValueKind == JsonValueKind.String) + { + result.PartitionKeyRangeId = pkRangeIdEl.GetString(); + } + + if (TryGetProperty(json, "requestCharge", out JsonElement requestChargeEl) && requestChargeEl.TryGetDouble(out double requestCharge)) + { + result.RequestCharge = requestCharge; + } + + if (TryGetProperty(json, DistributedTransactionSerializer.ResourceBody, out JsonElement resourceBody) && resourceBody.ValueKind != JsonValueKind.Undefined && resourceBody.ValueKind != JsonValueKind.Null) { @@ -192,5 +154,20 @@ internal static DistributedTransactionOperationResult FromJson(JsonElement json) return result; } + + internal static bool TryGetProperty(JsonElement element, string propertyName, out JsonElement value) + { + foreach (JsonProperty prop in element.EnumerateObject()) + { + if (string.Equals(prop.Name, propertyName, StringComparison.OrdinalIgnoreCase)) + { + value = prop.Value; + return true; + } + } + + value = default; + return false; + } } } diff --git a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs index a75d7e84a5..20e37baf4b 100644 --- a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs +++ b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs @@ -32,21 +32,17 @@ class DistributedTransactionResponse : IReadOnlyList operations, CosmosSerializerCore serializer, - ITrace trace, Guid idempotencyToken, bool isRetriable = false) { this.Headers = headers; this.StatusCode = statusCode; this.SubStatusCode = subStatusCode; - this.ErrorMessage = errorMessage; this.Operations = operations; this.SerializerCore = serializer; - this.Trace = trace; this.IdempotencyToken = idempotencyToken; this.IsRetriable = isRetriable; } @@ -103,11 +99,6 @@ public virtual DistributedTransactionOperationResult this[int index] /// public virtual bool IsSuccessStatusCode => (int)this.StatusCode >= 200 && (int)this.StatusCode <= 299; - /// - /// Gets the error message associated with the distributed transaction response, if any. - /// - public virtual string ErrorMessage { get; } - /// /// Gets the number of operation results in the distributed transaction response. /// @@ -129,8 +120,6 @@ public virtual DistributedTransactionOperationResult this[int index] internal IReadOnlyList Operations { get; } - internal ITrace Trace { get; } - /// /// Returns an enumerator that iterates through the operation results. /// @@ -169,7 +158,7 @@ internal static async Task FromResponseMessageAs ITrace trace, CancellationToken cancellationToken) { - using (ITrace createResponseTrace = trace.StartChild("Create Distributed Transaction Response", TraceComponent.Batch, TraceLevel.Info)) + using (trace.StartChild("Create Distributed Transaction Response", TraceComponent.Batch, TraceLevel.Info)) { cancellationToken.ThrowIfCancellationRequested(); @@ -200,7 +189,6 @@ internal static async Task FromResponseMessageAs serverRequest, serializer, idempotencyToken, - createResponseTrace, cancellationToken); } @@ -208,11 +196,9 @@ internal static async Task FromResponseMessageAs response ??= new DistributedTransactionResponse( responseMessage.StatusCode, responseMessage.Headers.SubStatusCode, - responseMessage.ErrorMessage, responseMessage.Headers, serverRequest.Operations, serializer, - createResponseTrace, idempotencyToken); // Validate results count matches operations count @@ -229,15 +215,13 @@ internal static async Task FromResponseMessageAs return new DistributedTransactionResponse( HttpStatusCode.InternalServerError, SubStatusCodes.Unknown, - ClientResources.InvalidServerResponse, responseMessage.Headers, serverRequest.Operations, serializer, - createResponseTrace, idempotencyToken); } - response.CreateAndPopulateResults(serverRequest.Operations, createResponseTrace); + response.CreateAndPopulateResults(serverRequest.Operations); } return response; @@ -298,7 +282,6 @@ private static async Task PopulateFromJsonConten DistributedTransactionServerRequest serverRequest, CosmosSerializerCore serializer, Guid idempotencyToken, - ITrace trace, CancellationToken cancellationToken) { List results = new List(); @@ -321,14 +304,14 @@ private static async Task PopulateFromJsonConten { JsonElement root = responseJson.RootElement; - if (root.TryGetProperty("isRetriable", out JsonElement isRetriableElement) && + if (DistributedTransactionOperationResult.TryGetProperty(root, "isRetriable", out JsonElement isRetriableElement) && isRetriableElement.ValueKind == JsonValueKind.True) { isRetriable = true; } // Parse operation results from "operationResponses" array. - if (root.TryGetProperty("operationResponses", out JsonElement operationResponses) && + if (DistributedTransactionOperationResult.TryGetProperty(root, "operationResponses", out JsonElement operationResponses) && operationResponses.ValueKind == JsonValueKind.Array) { try @@ -338,8 +321,6 @@ private static async Task PopulateFromJsonConten cancellationToken.ThrowIfCancellationRequested(); DistributedTransactionOperationResult operationResult = DistributedTransactionOperationResult.FromJson(operationElement); - operationResult.Trace = trace; - operationResult.ActivityId = responseMessage.Headers.ActivityId; results.Add(operationResult); } } @@ -375,11 +356,9 @@ private static async Task PopulateFromJsonConten return new DistributedTransactionResponse( finalStatusCode, finalSubStatusCode, - responseMessage.ErrorMessage, responseMessage.Headers, serverRequest.Operations, serializer, - trace, idempotencyToken, isRetriable) { @@ -388,8 +367,7 @@ private static async Task PopulateFromJsonConten } private void CreateAndPopulateResults( - IReadOnlyList operations, - ITrace trace) + IReadOnlyList operations) { this.results = new List(operations.Count); @@ -398,8 +376,6 @@ private void CreateAndPopulateResults( this.results.Add(new DistributedTransactionOperationResult(this.StatusCode) { SubStatusCode = this.SubStatusCode, - ActivityId = this.ActivityId, - Trace = trace }); } } diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs index cf839d3f8f..a1159f176c 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs @@ -492,7 +492,7 @@ public async Task Count_ReturnsNumberOfParsedResults() // Operation result property deserialization [TestMethod] - [Description("SubStatusCodeValue deserializes from the 'subStatusCode' JSON property and is accessible as both uint and SubStatusCodes enum.")] + [Description("SubStatusCode deserializes from the 'subStatusCode' JSON property.")] public async Task FromResponseMessage_OperationResult_SubStatusCode_DeserializesCorrectly() { const uint expectedSubStatusCode = 449; // RetryWith @@ -508,10 +508,8 @@ public async Task FromResponseMessage_OperationResult_SubStatusCode_Deserializes NoOpTrace.Singleton, CancellationToken.None); - Assert.AreEqual(expectedSubStatusCode, response[0].SubStatusCodeValue, - "SubStatusCodeValue must equal the uint value from the JSON 'subStatusCode' field."); Assert.AreEqual((SubStatusCodes)expectedSubStatusCode, response[0].SubStatusCode, - "SubStatusCode enum must be the cast of the uint value."); + "SubStatusCode must equal the cast of the uint value from the JSON 'subStatusCode' field."); } [TestMethod] From 60491cf7aa89e852d1a4b4080ec5497be7e92974 Mon Sep 17 00:00:00 2001 From: Meghana-Palaparthi Date: Tue, 26 May 2026 13:30:27 -0500 Subject: [PATCH 2/3] Address feedback --- .../DistributedTransactionOperationResult.cs | 101 ++++++++++++++++-- .../DistributedTransactionResponse.cs | 56 +++++++++- .../DistributedTransactionSerializer.cs | 6 ++ .../DistributedTransactionResponseTests.cs | 85 ++++++++++++++- 4 files changed, 237 insertions(+), 11 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs index e7743dbac7..9fca32c47a 100644 --- a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs +++ b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs @@ -75,44 +75,49 @@ internal DistributedTransactionOperationResult() /// The deserialized operation result with a canonical session token. internal static DistributedTransactionOperationResult FromJson(JsonElement json) { + if (json.ValueKind != JsonValueKind.Object) + { + throw new JsonException($"DTC operation result must be a JSON object, but was '{json.ValueKind}'."); + } + DistributedTransactionOperationResult result = new DistributedTransactionOperationResult(); - if (TryGetProperty(json, "index", out JsonElement indexEl) && indexEl.TryGetInt32(out int index)) + if (TryGetInt32Property(json, DistributedTransactionSerializer.Index, out int index)) { result.Index = index; } - if (TryGetProperty(json, "statusCode", out JsonElement statusCodeEl) && statusCodeEl.TryGetInt32(out int statusCode)) + if (TryGetInt32Property(json, DistributedTransactionSerializer.StatusCode, out int statusCode)) { result.StatusCode = (HttpStatusCode)statusCode; } - if (TryGetProperty(json, "subStatusCode", out JsonElement subStatusEl) && subStatusEl.TryGetUInt32(out uint subStatus)) + if (TryGetUInt32Property(json, DistributedTransactionSerializer.SubStatusCode, out uint subStatus)) { result.SubStatusCode = (SubStatusCodes)subStatus; } - if (TryGetProperty(json, "etag", out JsonElement etagEl) && etagEl.ValueKind == JsonValueKind.String) + if (TryGetProperty(json, DistributedTransactionSerializer.ResponseETag, out JsonElement etagEl) && etagEl.ValueKind == JsonValueKind.String) { result.ETag = etagEl.GetString(); } - if (TryGetProperty(json, "sessionToken", out JsonElement sessionTokenEl) && sessionTokenEl.ValueKind == JsonValueKind.String) + if (TryGetProperty(json, DistributedTransactionSerializer.SessionToken, out JsonElement sessionTokenEl) && sessionTokenEl.ValueKind == JsonValueKind.String) { result.SessionToken = sessionTokenEl.GetString(); } - if (TryGetProperty(json, "partitionKeyRangeId", out JsonElement pkRangeIdEl) && pkRangeIdEl.ValueKind == JsonValueKind.String) + if (TryGetProperty(json, DistributedTransactionSerializer.PartitionKeyRangeId, out JsonElement pkRangeIdEl) && pkRangeIdEl.ValueKind == JsonValueKind.String) { result.PartitionKeyRangeId = pkRangeIdEl.GetString(); } - if (TryGetProperty(json, "requestCharge", out JsonElement requestChargeEl) && requestChargeEl.TryGetDouble(out double requestCharge)) + if (TryGetDoubleProperty(json, DistributedTransactionSerializer.RequestCharge, out double requestCharge)) { result.RequestCharge = requestCharge; } - if (TryGetProperty(json, DistributedTransactionSerializer.ResourceBody, out JsonElement resourceBody) + if (TryGetPropertyOrdinal(json, DistributedTransactionSerializer.ResourceBody, out JsonElement resourceBody) && resourceBody.ValueKind != JsonValueKind.Undefined && resourceBody.ValueKind != JsonValueKind.Null) { @@ -157,6 +162,12 @@ internal static DistributedTransactionOperationResult FromJson(JsonElement json) internal static bool TryGetProperty(JsonElement element, string propertyName, out JsonElement value) { + if (element.ValueKind != JsonValueKind.Object) + { + value = default; + return false; + } + foreach (JsonProperty prop in element.EnumerateObject()) { if (string.Equals(prop.Name, propertyName, StringComparison.OrdinalIgnoreCase)) @@ -169,5 +180,79 @@ internal static bool TryGetProperty(JsonElement element, string propertyName, ou value = default; return false; } + + internal static bool TryGetPropertyOrdinal(JsonElement element, string propertyName, out JsonElement value) + { + if (element.ValueKind != JsonValueKind.Object) + { + value = default; + return false; + } + + return element.TryGetProperty(propertyName, out value); + } + + private static bool TryGetInt32Property(JsonElement element, string propertyName, out int value) + { + if (TryGetProperty(element, propertyName, out JsonElement propertyElement)) + { + if (propertyElement.ValueKind != JsonValueKind.Number) + { + throw new JsonException($"'{propertyName}' must be a JSON number, but was '{propertyElement.ValueKind}'."); + } + + if (!propertyElement.TryGetInt32(out value)) + { + throw new JsonException($"'{propertyName}' must be a 32-bit integer JSON number."); + } + + return true; + } + + value = default; + return false; + } + + private static bool TryGetUInt32Property(JsonElement element, string propertyName, out uint value) + { + if (TryGetProperty(element, propertyName, out JsonElement propertyElement)) + { + if (propertyElement.ValueKind != JsonValueKind.Number) + { + throw new JsonException($"'{propertyName}' must be a JSON number, but was '{propertyElement.ValueKind}'."); + } + + if (!propertyElement.TryGetUInt32(out value)) + { + throw new JsonException($"'{propertyName}' must be a 32-bit unsigned integer JSON number."); + } + + return true; + } + + value = default; + return false; + } + + private static bool TryGetDoubleProperty(JsonElement element, string propertyName, out double value) + { + if (TryGetProperty(element, propertyName, out JsonElement propertyElement)) + { + if (propertyElement.ValueKind != JsonValueKind.Number) + { + throw new JsonException($"'{propertyName}' must be a JSON number, but was '{propertyElement.ValueKind}'."); + } + + if (!propertyElement.TryGetDouble(out value)) + { + throw new JsonException($"'{propertyName}' must be a double-precision JSON number."); + } + + return true; + } + + value = default; + return false; + } } } diff --git a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs index 20e37baf4b..eb34e3b1bc 100644 --- a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs +++ b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs @@ -32,6 +32,7 @@ class DistributedTransactionResponse : IReadOnlyList operations, CosmosSerializerCore serializer, @@ -41,6 +42,7 @@ private DistributedTransactionResponse( this.Headers = headers; this.StatusCode = statusCode; this.SubStatusCode = subStatusCode; + this.ErrorMessage = errorMessage; this.Operations = operations; this.SerializerCore = serializer; this.IdempotencyToken = idempotencyToken; @@ -99,6 +101,11 @@ public virtual DistributedTransactionOperationResult this[int index] /// public virtual bool IsSuccessStatusCode => (int)this.StatusCode >= 200 && (int)this.StatusCode <= 299; + /// + /// Gets the error message associated with the distributed transaction response. + /// + public virtual string ErrorMessage { get; } + /// /// Gets the number of operation results in the distributed transaction response. /// @@ -196,6 +203,7 @@ internal static async Task FromResponseMessageAs response ??= new DistributedTransactionResponse( responseMessage.StatusCode, responseMessage.Headers.SubStatusCode, + responseMessage.ErrorMessage, responseMessage.Headers, serverRequest.Operations, serializer, @@ -215,6 +223,7 @@ internal static async Task FromResponseMessageAs return new DistributedTransactionResponse( HttpStatusCode.InternalServerError, SubStatusCodes.Unknown, + ClientResources.InvalidServerResponse, responseMessage.Headers, serverRequest.Operations, serializer, @@ -297,6 +306,12 @@ private static async Task PopulateFromJsonConten DefaultTrace.TraceWarning( "DistributedTransactionResponse: failed to parse response body: {0}", jsonEx.Message); + + if (responseMessage.IsSuccessStatusCode) + { + return CreateDeserializationFailureResponse(responseMessage, serverRequest, serializer, idempotencyToken); + } + return null; } @@ -304,14 +319,14 @@ private static async Task PopulateFromJsonConten { JsonElement root = responseJson.RootElement; - if (DistributedTransactionOperationResult.TryGetProperty(root, "isRetriable", out JsonElement isRetriableElement) && + if (DistributedTransactionOperationResult.TryGetPropertyOrdinal(root, DistributedTransactionSerializer.IsRetriable, out JsonElement isRetriableElement) && isRetriableElement.ValueKind == JsonValueKind.True) { isRetriable = true; } // Parse operation results from "operationResponses" array. - if (DistributedTransactionOperationResult.TryGetProperty(root, "operationResponses", out JsonElement operationResponses) && + if (DistributedTransactionOperationResult.TryGetPropertyOrdinal(root, DistributedTransactionSerializer.OperationResponses, out JsonElement operationResponses) && operationResponses.ValueKind == JsonValueKind.Array) { try @@ -329,8 +344,21 @@ private static async Task PopulateFromJsonConten DefaultTrace.TraceWarning( "DistributedTransactionResponse: per-operation parse failed; forcing isRetriable=false. {0}", jsonEx.Message); + + // Dispose any resource streams allocated for the partially-parsed operations + // before discarding them. + foreach (DistributedTransactionOperationResult partial in results) + { + partial.ResourceStream?.Dispose(); + } + results.Clear(); isRetriable = false; + + if (responseMessage.IsSuccessStatusCode) + { + return CreateDeserializationFailureResponse(responseMessage, serverRequest, serializer, idempotencyToken); + } } } } @@ -356,6 +384,7 @@ private static async Task PopulateFromJsonConten return new DistributedTransactionResponse( finalStatusCode, finalSubStatusCode, + responseMessage.ErrorMessage, responseMessage.Headers, serverRequest.Operations, serializer, @@ -379,5 +408,28 @@ private void CreateAndPopulateResults( }); } } + + /// + /// Builds an InternalServerError response indicating the server replied with success but + /// the SDK could not deserialize the response payload. Mirrors TransactionalBatch behavior. + /// + private static DistributedTransactionResponse CreateDeserializationFailureResponse( + ResponseMessage responseMessage, + DistributedTransactionServerRequest serverRequest, + CosmosSerializerCore serializer, + Guid idempotencyToken) + { + DistributedTransactionResponse failedResponse = new DistributedTransactionResponse( + HttpStatusCode.InternalServerError, + SubStatusCodes.Unknown, + ClientResources.ServerResponseDeserializationFailure, + responseMessage.Headers, + serverRequest.Operations, + serializer, + idempotencyToken); + + failedResponse.CreateAndPopulateResults(serverRequest.Operations); + return failedResponse; + } } } diff --git a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionSerializer.cs b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionSerializer.cs index c35f49d64c..bb4f9416f1 100644 --- a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionSerializer.cs +++ b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionSerializer.cs @@ -24,10 +24,16 @@ internal static class DistributedTransactionSerializer internal const string DatabaseResourceId = "databaseResourceId"; internal const string PartitionKey = "partitionKey"; internal const string Index = "index"; + internal const string StatusCode = "statusCode"; + internal const string SubStatusCode = "subStatusCode"; internal const string ResourceBody = "resourceBody"; + internal const string RequestCharge = "requestCharge"; + internal const string IsRetriable = "isRetriable"; + internal const string OperationResponses = "operationResponses"; internal const string SessionToken = "sessionToken"; internal const string PartitionKeyRangeId = "partitionKeyRangeId"; internal const string ETag = "ifMatch"; + internal const string ResponseETag = "Etag"; internal const string OperationType = "operationType"; internal const string ResourceType = "resourceType"; diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs index a1159f176c..ea69823651 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs @@ -100,7 +100,7 @@ public async Task FromResponseMessage_PreconditionFailed_ReturnsFailureStatus() } [TestMethod] - [Description("When the response body contains malformed JSON and the HTTP status is success, the SDK must return 500.")] + [Description("When the response body contains malformed JSON and the HTTP status is success, the SDK must return 500 with a deserialization-failure error message.")] public async Task FromResponseMessage_MalformedJson_SuccessStatus_ReturnsInternalServerError() { DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); @@ -116,6 +116,7 @@ public async Task FromResponseMessage_MalformedJson_SuccessStatus_ReturnsInterna Assert.AreEqual(HttpStatusCode.InternalServerError, response.StatusCode); Assert.IsFalse(response.IsSuccessStatusCode); + Assert.AreEqual(ClientResources.ServerResponseDeserializationFailure, response.ErrorMessage); } [TestMethod] @@ -142,6 +143,68 @@ public async Task FromResponseMessage_MalformedJson_ErrorStatus_PopulatesResults } } + [DataTestMethod] + [Description("When numeric operation fields are present but not numeric, per-operation parsing fails and a success response is converted to InternalServerError with a deserialization-failure message.")] + [DataRow(@"{""operationResponses"":[{""index"":0,""statusCode"":""abc""}]}", DisplayName = "statusCode wrong type")] + [DataRow(@"{""operationResponses"":[{""index"":0,""statusCode"":449,""subStatusCode"":""abc""}]}", DisplayName = "subStatusCode wrong type")] + [DataRow(@"{""operationResponses"":[{""index"":0,""statusCode"":201,""requestCharge"":""abc""}]}", DisplayName = "requestCharge wrong type")] + [DataRow(@"{""operationResponses"":[{""index"":""abc"",""statusCode"":201}]}", DisplayName = "index wrong type")] + public async Task FromResponseMessage_OperationResult_NumericTypeMismatch_SuccessStatus_ReturnsInternalServerError(string json) + { + DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); + ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); + + DistributedTransactionResponse response = await DistributedTransactionResponse.FromResponseMessageAsync( + responseMessage, + serverRequest, + MockCosmosUtil.Serializer, + NoOpTrace.Singleton, + CancellationToken.None); + + Assert.AreEqual(HttpStatusCode.InternalServerError, response.StatusCode); + Assert.IsFalse(response.IsSuccessStatusCode); + Assert.AreEqual(ClientResources.ServerResponseDeserializationFailure, response.ErrorMessage); + } + + [TestMethod] + [Description("When operationResponses contains a non-object entry, the parser fails safely and a success response is converted to InternalServerError with a deserialization-failure message.")] + public async Task FromResponseMessage_OperationResult_NonObjectElement_SuccessStatus_ReturnsInternalServerError() + { + DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); + string json = @"{""operationResponses"":[null]}"; + ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); + + DistributedTransactionResponse response = await DistributedTransactionResponse.FromResponseMessageAsync( + responseMessage, + serverRequest, + MockCosmosUtil.Serializer, + NoOpTrace.Singleton, + CancellationToken.None); + + Assert.AreEqual(HttpStatusCode.InternalServerError, response.StatusCode); + Assert.IsFalse(response.IsSuccessStatusCode); + Assert.AreEqual(ClientResources.ServerResponseDeserializationFailure, response.ErrorMessage); + } + + [TestMethod] + [Description("Top-level lookups are case-sensitive; PascalCase 'OperationResponses' is ignored and success responses become InternalServerError due to count mismatch.")] + public async Task FromResponseMessage_OperationResponses_PascalCaseKey_SuccessStatus_ReturnsInternalServerError() + { + DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); + string json = @"{""OperationResponses"":[{""index"":0,""statusCode"":201}]}"; + ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); + + DistributedTransactionResponse response = await DistributedTransactionResponse.FromResponseMessageAsync( + responseMessage, + serverRequest, + MockCosmosUtil.Serializer, + NoOpTrace.Singleton, + CancellationToken.None); + + Assert.AreEqual(HttpStatusCode.InternalServerError, response.StatusCode); + Assert.IsFalse(response.IsSuccessStatusCode); + } + // Count mismatch [TestMethod] @@ -554,6 +617,25 @@ public async Task FromResponseMessage_OperationResult_ETag_DeserializesCorrectly "ETag must equal the value from the JSON 'etag' field."); } + [TestMethod] + [Description("resourceBody lookup remains case-sensitive; PascalCase 'ResourceBody' must not populate ResourceStream.")] + public async Task FromResponseMessage_OperationResult_ResourceBody_PascalCaseKey_DoesNotDeserialize() + { + DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); + + string json = @"{""operationResponses"":[{""index"":0,""statusCode"":201,""ResourceBody"":{""id"":""item1""}}]}"; + ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); + + DistributedTransactionResponse response = await DistributedTransactionResponse.FromResponseMessageAsync( + responseMessage, + serverRequest, + MockCosmosUtil.Serializer, + NoOpTrace.Singleton, + CancellationToken.None); + + Assert.IsNull(response[0].ResourceStream, "ResourceStream must remain null when the property name is not exact 'resourceBody'."); + } + [TestMethod] [Description("SessionToken is assembled as {pkRangeId}:{lsn} from the separate 'sessionToken' (LSN-only) and 'partitionKeyRangeId' JSON fields.")] public async Task FromResponseMessage_OperationResult_SessionToken_DeserializesCorrectly() @@ -722,6 +804,7 @@ public async Task FromResponseMessage_OperationResult_SessionToken_NullWhenSessi [DataRow(@"{""isRetriable"":false,""operationResponses"":[{""index"":0,""statusCode"":503}]}", false, DisplayName = "JSON boolean false → IsRetriable=false")] [DataRow(@"{""operationResponses"":[{""index"":0,""statusCode"":503}]}", false, DisplayName = "isRetriable absent → IsRetriable=false")] [DataRow(@"{""isRetriable"":""true"",""operationResponses"":[{""index"":0,""statusCode"":503}]}", false, DisplayName = "string 'true' (not a JSON boolean) → IsRetriable=false")] + [DataRow(@"{""IsRetriable"":true,""operationResponses"":[{""index"":0,""statusCode"":503}]}", false, DisplayName = "PascalCase 'IsRetriable' is ignored")] public async Task FromResponseMessage_IsRetriable_Parsing(string json, bool expected) { DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); From ba40a481af32c037a8ff994bbd01a8f31ecdfd42 Mon Sep 17 00:00:00 2001 From: Meghana-Palaparthi Date: Thu, 28 May 2026 13:13:50 -0500 Subject: [PATCH 3/3] Make all property name lookups case-insensitive, remove dead STJ attributes, consolidate tests - All property name lookups (including resourceBody) now use case-insensitive TryGetProperty; removed unused TryGetPropertyOrdinal method - Top-level response properties (isRetriable, diagnosticString, operationResponses) switched from ordinal to case-insensitive lookup - Removed [JsonIgnore], [JsonConstructor] attributes and System.Text.Json.Serialization using (dead code since manual FromJson parsing) - Changed parameterless constructor from public back to internal - Consolidated repetitive tests into DataTestMethods (NumericTypeMismatch + NonObjectElement, Indexer bounds, IdempotencyToken fallback) - Removed redundant tests (Dispose_ReleasesResultResourceStreams, FollowedByDirectStreamRead) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DistributedTransactionOperationResult.cs | 27 +-- .../DistributedTransactionResponse.cs | 6 +- .../DistributedTransactionResponseTests.cs | 164 ++++-------------- 3 files changed, 40 insertions(+), 157 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs index d10baf36a4..dc5b06e131 100644 --- a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs +++ b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionOperationResult.cs @@ -8,7 +8,6 @@ namespace Microsoft.Azure.Cosmos using System.IO; using System.Net; using System.Text.Json; - using System.Text.Json.Serialization; using Microsoft.Azure.Cosmos.Core.Trace; using Microsoft.Azure.Cosmos.Tracing; using Microsoft.Azure.Documents; @@ -46,14 +45,7 @@ internal DistributedTransactionOperationResult(DistributedTransactionOperationRe /// /// Initializes a new instance of the class. /// - /// - /// Must be public for System.Text.Json reflection-based deserialization. - /// System.Text.Json 6.x only scans BindingFlags.Public constructors when resolving - /// ; non-public constructors are not found. - /// Support for non-public constructors was added in System.Text.Json 7.0. - /// - [JsonConstructor] - public DistributedTransactionOperationResult() + internal DistributedTransactionOperationResult() { } @@ -88,7 +80,6 @@ public DistributedTransactionOperationResult() /// Do not dispose it directly. To deserialize to a typed object, use /// . /// - [JsonIgnore] public virtual Stream ResourceStream { get; internal set; } /// @@ -105,13 +96,10 @@ public DistributedTransactionOperationResult() /// /// ActivityId related to the operation. /// - [JsonIgnore] internal virtual string ActivityId { get; set; } - [JsonIgnore] internal ITrace Trace { get; set; } - [JsonIgnore] internal CosmosSerializerCore SerializerCore { get; set; } /// @@ -203,7 +191,7 @@ internal static DistributedTransactionOperationResult FromJson(JsonElement json) result.RequestCharge = requestCharge; } - if (TryGetPropertyOrdinal(json, DistributedTransactionSerializer.ResourceBody, out JsonElement resourceBody) + if (TryGetProperty(json, DistributedTransactionSerializer.ResourceBody, out JsonElement resourceBody) && resourceBody.ValueKind != JsonValueKind.Undefined && resourceBody.ValueKind != JsonValueKind.Null) { @@ -267,17 +255,6 @@ internal static bool TryGetProperty(JsonElement element, string propertyName, ou return false; } - internal static bool TryGetPropertyOrdinal(JsonElement element, string propertyName, out JsonElement value) - { - if (element.ValueKind != JsonValueKind.Object) - { - value = default; - return false; - } - - return element.TryGetProperty(propertyName, out value); - } - private static bool TryGetInt32Property(JsonElement element, string propertyName, out int value) { if (TryGetProperty(element, propertyName, out JsonElement propertyElement)) diff --git a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs index 7e1dd37aa5..21d55e57ad 100644 --- a/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs +++ b/Microsoft.Azure.Cosmos/src/DistributedTransaction/DistributedTransactionResponse.cs @@ -375,20 +375,20 @@ private static async Task PopulateFromJsonConten { JsonElement root = responseJson.RootElement; - if (DistributedTransactionOperationResult.TryGetPropertyOrdinal(root, DistributedTransactionSerializer.IsRetriable, out JsonElement isRetriableElement) && + if (DistributedTransactionOperationResult.TryGetProperty(root, DistributedTransactionSerializer.IsRetriable, out JsonElement isRetriableElement) && isRetriableElement.ValueKind == JsonValueKind.True) { isRetriable = true; } - if (root.TryGetProperty(DistributedTransactionSerializer.DiagnosticString, out JsonElement diagnosticStringElement) && + if (DistributedTransactionOperationResult.TryGetProperty(root, DistributedTransactionSerializer.DiagnosticString, out JsonElement diagnosticStringElement) && diagnosticStringElement.ValueKind == JsonValueKind.String) { diagnosticString = diagnosticStringElement.GetString(); } // Parse operation results from "operationResponses" array. - if (DistributedTransactionOperationResult.TryGetPropertyOrdinal(root, DistributedTransactionSerializer.OperationResponses, out JsonElement operationResponses) && + if (DistributedTransactionOperationResult.TryGetProperty(root, DistributedTransactionSerializer.OperationResponses, out JsonElement operationResponses) && operationResponses.ValueKind == JsonValueKind.Array) { try diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs index 58b94b5787..66aa6bbce9 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/DistributedTransaction/DistributedTransactionResponseTests.cs @@ -144,12 +144,13 @@ public async Task FromResponseMessage_MalformedJson_ErrorStatus_PopulatesResults } [DataTestMethod] - [Description("When numeric operation fields are present but not numeric, per-operation parsing fails and a success response is converted to InternalServerError with a deserialization-failure message.")] + [Description("When per-operation fields have wrong types or non-object entries, parsing fails and a success response is converted to InternalServerError with a deserialization-failure message.")] [DataRow(@"{""operationResponses"":[{""index"":0,""statusCode"":""abc""}]}", DisplayName = "statusCode wrong type")] [DataRow(@"{""operationResponses"":[{""index"":0,""statusCode"":449,""subStatusCode"":""abc""}]}", DisplayName = "subStatusCode wrong type")] [DataRow(@"{""operationResponses"":[{""index"":0,""statusCode"":201,""requestCharge"":""abc""}]}", DisplayName = "requestCharge wrong type")] [DataRow(@"{""operationResponses"":[{""index"":""abc"",""statusCode"":201}]}", DisplayName = "index wrong type")] - public async Task FromResponseMessage_OperationResult_NumericTypeMismatch_SuccessStatus_ReturnsInternalServerError(string json) + [DataRow(@"{""operationResponses"":[null]}", DisplayName = "non-object element (null)")] + public async Task FromResponseMessage_OperationResult_InvalidElement_SuccessStatus_ReturnsInternalServerError(string json) { DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); @@ -167,28 +168,8 @@ public async Task FromResponseMessage_OperationResult_NumericTypeMismatch_Succes } [TestMethod] - [Description("When operationResponses contains a non-object entry, the parser fails safely and a success response is converted to InternalServerError with a deserialization-failure message.")] - public async Task FromResponseMessage_OperationResult_NonObjectElement_SuccessStatus_ReturnsInternalServerError() - { - DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); - string json = @"{""operationResponses"":[null]}"; - ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); - - DistributedTransactionResponse response = await DistributedTransactionResponse.FromResponseMessageAsync( - responseMessage, - serverRequest, - MockCosmosUtil.Serializer, - NoOpTrace.Singleton, - CancellationToken.None); - - Assert.AreEqual(HttpStatusCode.InternalServerError, response.StatusCode); - Assert.IsFalse(response.IsSuccessStatusCode); - Assert.AreEqual(ClientResources.ServerResponseDeserializationFailure, response.ErrorMessage); - } - - [TestMethod] - [Description("Top-level lookups are case-sensitive; PascalCase 'OperationResponses' is ignored and success responses become InternalServerError due to count mismatch.")] - public async Task FromResponseMessage_OperationResponses_PascalCaseKey_SuccessStatus_ReturnsInternalServerError() + [Description("Top-level lookups are case-insensitive; PascalCase 'OperationResponses' is accepted.")] + public async Task FromResponseMessage_OperationResponses_PascalCaseKey_SuccessStatus_ParsesSuccessfully() { DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); string json = @"{""OperationResponses"":[{""index"":0,""statusCode"":201}]}"; @@ -201,8 +182,10 @@ public async Task FromResponseMessage_OperationResponses_PascalCaseKey_SuccessSt NoOpTrace.Singleton, CancellationToken.None); - Assert.AreEqual(HttpStatusCode.InternalServerError, response.StatusCode); - Assert.IsFalse(response.IsSuccessStatusCode); + Assert.AreEqual(HttpStatusCode.OK, response.StatusCode); + Assert.IsTrue(response.IsSuccessStatusCode); + Assert.AreEqual(1, response.Count); + Assert.AreEqual(HttpStatusCode.Created, response[0].StatusCode); } // Count mismatch @@ -347,36 +330,21 @@ public async Task FromResponseMessage_MultiStatus_AllFailedDependency_StatusRema // Idempotency token resolution - [TestMethod] - [Description("When the IdempotencyToken header is absent from the response, the request token is used as the fallback.")] - public async Task FromResponseMessage_IdempotencyToken_MissingFromHeader_FallsBackToRequestToken() + [DataTestMethod] + [Description("When the IdempotencyToken response header is absent or unparseable, the SDK falls back to the request token.")] + [DataRow(null, DisplayName = "Header absent")] + [DataRow("not-a-valid-guid", DisplayName = "Invalid GUID in header")] + public async Task FromResponseMessage_IdempotencyToken_FallsBackToRequestToken(string headerValue) { DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); string json = @"{""operationResponses"":[{""index"":0,""statusCode"":201}]}"; ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); - // No IdempotencyToken header added - - DistributedTransactionResponse response = await DistributedTransactionResponse.FromResponseMessageAsync( - responseMessage, - serverRequest, - MockCosmosUtil.Serializer, - NoOpTrace.Singleton, - CancellationToken.None); - - Assert.AreEqual(serverRequest.IdempotencyToken, response.IdempotencyToken, - "The request token must be used when the response header is absent."); - } - - [TestMethod] - [Description("When the IdempotencyToken response header contains a non-GUID value, the SDK falls back to the request token.")] - public async Task FromResponseMessage_IdempotencyToken_InvalidGuidInHeader_FallsBackToRequestToken() - { - DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); - string json = @"{""operationResponses"":[{""index"":0,""statusCode"":201}]}"; - ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); - responseMessage.Headers.Add(HttpConstants.HttpHeaders.IdempotencyToken, "not-a-valid-guid"); + if (headerValue != null) + { + responseMessage.Headers.Add(HttpConstants.HttpHeaders.IdempotencyToken, headerValue); + } DistributedTransactionResponse response = await DistributedTransactionResponse.FromResponseMessageAsync( responseMessage, @@ -386,34 +354,11 @@ public async Task FromResponseMessage_IdempotencyToken_InvalidGuidInHeader_Falls CancellationToken.None); Assert.AreEqual(serverRequest.IdempotencyToken, response.IdempotencyToken, - "An unparseable header value must fall back to the request token."); + "The request token must be used when the response header is absent or unparseable."); } // IDisposable and ObjectDisposed - [TestMethod] - [Description("Dispose() must set result ResourceStreams to null so callers cannot accidentally use a closed stream.")] - public async Task Dispose_ReleasesResultResourceStreams() - { - DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); - - string json = @"{""operationResponses"":[{""index"":0,""statusCode"":201,""resourceBody"":{""id"":""item1""}}]}"; - ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); - - DistributedTransactionResponse response = await DistributedTransactionResponse.FromResponseMessageAsync( - responseMessage, - serverRequest, - MockCosmosUtil.Serializer, - NoOpTrace.Singleton, - CancellationToken.None); - - Assert.IsNotNull(response[0].ResourceStream, "ResourceStream should be populated from resourcebody before Dispose."); - - response.Dispose(); - - // Indexer-after-dispose behavior is covered by Indexer_AfterDispose_ThrowsObjectDisposedException. - } - [TestMethod] [Description("Calling Dispose() a second time must be a safe no-op.")] public async Task Dispose_SecondCall_DoesNotThrow() @@ -491,27 +436,11 @@ public async Task Indexer_ValidIndex_ReturnsExpectedResult() Assert.AreEqual(1, response[1].Index); } - [TestMethod] - [Description("Accessing a negative index must throw ArgumentOutOfRangeException.")] - public async Task Indexer_NegativeIndex_ThrowsArgumentOutOfRangeException() - { - DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); - string json = @"{""operationResponses"":[{""index"":0,""statusCode"":201}]}"; - ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); - - DistributedTransactionResponse response = await DistributedTransactionResponse.FromResponseMessageAsync( - responseMessage, - serverRequest, - MockCosmosUtil.Serializer, - NoOpTrace.Singleton, - CancellationToken.None); - - Assert.ThrowsException(() => _ = response[-1]); - } - - [TestMethod] - [Description("Accessing index equal to Count must throw ArgumentOutOfRangeException.")] - public async Task Indexer_IndexEqualsCount_ThrowsArgumentOutOfRangeException() + [DataTestMethod] + [Description("Accessing an out-of-range index must throw ArgumentOutOfRangeException.")] + [DataRow(-1, DisplayName = "Negative index")] + [DataRow(1, DisplayName = "Index equals count")] + public async Task Indexer_OutOfRange_ThrowsArgumentOutOfRangeException(int index) { DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); string json = @"{""operationResponses"":[{""index"":0,""statusCode"":201}]}"; @@ -524,7 +453,7 @@ public async Task Indexer_IndexEqualsCount_ThrowsArgumentOutOfRangeException() NoOpTrace.Singleton, CancellationToken.None); - Assert.ThrowsException(() => _ = response[response.Count]); + Assert.ThrowsException(() => _ = response[index]); } [TestMethod] @@ -641,8 +570,8 @@ public async Task FromResponseMessage_OperationResult_ETag_DeserializesCorrectly } [TestMethod] - [Description("resourceBody lookup remains case-sensitive; PascalCase 'ResourceBody' must not populate ResourceStream.")] - public async Task FromResponseMessage_OperationResult_ResourceBody_PascalCaseKey_DoesNotDeserialize() + [Description("resourceBody lookup is case-insensitive; PascalCase 'ResourceBody' populates ResourceStream.")] + public async Task FromResponseMessage_OperationResult_ResourceBody_PascalCaseKey_Deserializes() { DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); @@ -656,7 +585,7 @@ public async Task FromResponseMessage_OperationResult_ResourceBody_PascalCaseKey NoOpTrace.Singleton, CancellationToken.None); - Assert.IsNull(response[0].ResourceStream, "ResourceStream must remain null when the property name is not exact 'resourceBody'."); + Assert.IsNotNull(response[0].ResourceStream, "ResourceStream should be populated because property name lookup is case-insensitive."); } [TestMethod] @@ -827,7 +756,7 @@ public async Task FromResponseMessage_OperationResult_SessionToken_NullWhenSessi [DataRow(@"{""isRetriable"":false,""operationResponses"":[{""index"":0,""statusCode"":503}]}", false, DisplayName = "JSON boolean false → IsRetriable=false")] [DataRow(@"{""operationResponses"":[{""index"":0,""statusCode"":503}]}", false, DisplayName = "isRetriable absent → IsRetriable=false")] [DataRow(@"{""isRetriable"":""true"",""operationResponses"":[{""index"":0,""statusCode"":503}]}", false, DisplayName = "string 'true' (not a JSON boolean) → IsRetriable=false")] - [DataRow(@"{""IsRetriable"":true,""operationResponses"":[{""index"":0,""statusCode"":503}]}", false, DisplayName = "PascalCase 'IsRetriable' is ignored")] + [DataRow(@"{""IsRetriable"":true,""operationResponses"":[{""index"":0,""statusCode"":503}]}", true, DisplayName = "PascalCase 'IsRetriable' is accepted")] public async Task FromResponseMessage_IsRetriable_Parsing(string json, bool expected) { DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); @@ -845,14 +774,16 @@ public async Task FromResponseMessage_IsRetriable_Parsing(string json, bool expe // DiagnosticString parsing - [TestMethod] - [Description("DiagnosticString deserializes from the 'diagnosticString' JSON property.")] - public async Task FromResponseMessage_DiagnosticString_DeserializesCorrectly() + [DataTestMethod] + [Description("DiagnosticString deserializes from the top-level JSON property case-insensitively.")] + [DataRow("diagnosticString", DisplayName = "camelCase key")] + [DataRow("DiagnosticString", DisplayName = "PascalCase key")] + public async Task FromResponseMessage_DiagnosticString_DeserializesCorrectly(string diagnosticStringPropertyName) { const string expectedDiagnosticString = "TransactionCommitted"; DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); - string json = $@"{{""diagnosticString"":""{expectedDiagnosticString}"",""operationResponses"":[{{""index"":0,""statusCode"":200}}]}}"; + string json = $@"{{""{diagnosticStringPropertyName}"":""{expectedDiagnosticString}"",""operationResponses"":[{{""index"":0,""statusCode"":200}}]}}"; ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); DistributedTransactionResponse response = await DistributedTransactionResponse.FromResponseMessageAsync( @@ -1277,31 +1208,6 @@ public async Task GetOperationResultAtIndex_CalledTwice_BothCallsSucceedAndStrea StringAssert.Contains(raw, "\"id\":\"dup\"", "Underlying ResourceStream must remain readable after GetOperationResultAtIndex."); } - [TestMethod] - [Description("Reading response[index].ResourceStream directly after GetOperationResultAtIndex must still return the original bytes.")] - public async Task GetOperationResultAtIndex_FollowedByDirectStreamRead_StreamIsIntact() - { - DistributedTransactionServerRequest serverRequest = await BuildServerRequestAsync(operationCount: 1); - string json = @"{""operationResponses"":[{""index"":0,""statusCode"":200,""resourceBody"":{""id"":""x"",""value"":99}}]}"; - ResponseMessage responseMessage = BuildResponseMessage(HttpStatusCode.OK, json); - - DistributedTransactionResponse response = await DistributedTransactionResponse.FromResponseMessageAsync( - responseMessage, - serverRequest, - MockCosmosUtil.Serializer, - NoOpTrace.Singleton, - CancellationToken.None); - - _ = response.GetOperationResultAtIndex(0); - - Stream stream = response[0].ResourceStream; - Assert.IsNotNull(stream); - stream.Position = 0; - using StreamReader reader = new StreamReader(stream); - string raw = reader.ReadToEnd(); - Assert.AreEqual(@"{""id"":""x"",""value"":99}", raw); - } - [TestMethod] [Description("Calling Dispose() after GetOperationResultAtIndex must still successfully release the ResourceStream and must be safe to call multiple times.")] public async Task GetOperationResultAtIndex_FollowedByDispose_DoesNotThrow()