From 44f7cd19a69b6f9f05657e2656295ae7a38e9b34 Mon Sep 17 00:00:00 2001 From: philipthomas Date: Wed, 27 Dec 2023 13:26:34 -0500 Subject: [PATCH 01/15] initial commit --- .../src/GatewayStoreClient.cs | 34 ++++++- .../GatewayStoreClientTests.cs | 98 +++++++++++++++++++ 2 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index e151ffcd94..e74ef4be59 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -9,6 +9,7 @@ namespace Microsoft.Azure.Cosmos using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.IO; + using System.Linq; using System.Net.Http; using System.Net.Http.Headers; using System.Text; @@ -150,8 +151,26 @@ internal static async Task CreateDocumentClientExceptio // If service rejects the initial payload like header is to large it will return an HTML error instead of JSON. if (string.Equals(responseMessage.Content?.Headers?.ContentType?.MediaType, "application/json", StringComparison.OrdinalIgnoreCase)) { + // For more information, see https://github.com/Azure/azure-cosmos-dotnet-v3/issues/4162. + if (await GatewayStoreClient.IsJsonHTTPResponseFromGatewayInvalidAsync(responseMessage)) + { + return new DocumentClientException( + new Error + { + Code = responseMessage.StatusCode.ToString(), + Message = "Invalid JSON HTTP response from Gateway." + }, + responseMessage.Headers, + responseMessage.StatusCode) + { + StatusDescription = responseMessage.ReasonPhrase, + ResourceAddress = resourceIdOrFullName, + RequestStatistics = requestStatistics + }; + } + Stream readStream = await responseMessage.Content.ReadAsStreamAsync(); - Error error = Documents.Resource.LoadFrom(readStream); + Error error = Resource.LoadFrom(readStream); return new DocumentClientException( error, responseMessage.Headers, @@ -197,6 +216,19 @@ internal static async Task CreateDocumentClientExceptio } } + /// + /// Checking if exception response (deserializable Error object) is valid based on the content length. + /// For more information, see . + /// + /// + private static async Task IsJsonHTTPResponseFromGatewayInvalidAsync(HttpResponseMessage responseMessage) + { + string readString = await responseMessage.Content.ReadAsStringAsync(); + + return responseMessage.Content?.Headers?.ContentLength == 0 || + readString.Trim().Length == 0; + } + internal static bool IsAllowedRequestHeader(string headerName) { if (!headerName.StartsWith("x-ms", StringComparison.OrdinalIgnoreCase)) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs new file mode 100644 index 0000000000..581d4ed0d7 --- /dev/null +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs @@ -0,0 +1,98 @@ +//------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. All rights reserved. +//------------------------------------------------------------ +namespace Microsoft.Azure.Cosmos +{ + using System; + using System.Net; + using System.Net.Http; + using System.Threading.Tasks; + using Microsoft.Azure.Cosmos.Tracing; + using Microsoft.Azure.Cosmos.Tracing.TraceData; + using Microsoft.Azure.Documents; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using Newtonsoft.Json; + + /// + /// Tests for . + /// + [TestClass] + public class GatewayStoreClientTests + { + /// + /// Testing the exception behavior when a response from the Gateway has no response (deserializable Error object) based on the content length. + /// For more information, see . + /// + /// + [TestMethod] + [DataRow(@"")] + [DataRow(@" ")] + public async Task CreateDocumentClientExceptionInvalidJsonResponseFromGatewayTestAsync(string content) + { + HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + { + RequestMessage = new HttpRequestMessage( + method: HttpMethod.Get, + requestUri: @"https://pt_ac_test_uri.com/"), + Content = new StringContent( + content: content), + }; + + responseMessage.Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/json"); + + IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( + startTime: DateTime.UtcNow, + trace: NoOpTrace.Singleton); + + DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( + responseMessage: responseMessage, + requestStatistics: requestStatistics); + + Assert.IsNotNull(value: documentClientException); + Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); + Assert.IsTrue(condition: documentClientException.Message.Contains("Invalid JSON HTTP response from Gateway.")); + + Assert.IsNotNull(value: documentClientException.Error); + Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); + Assert.AreEqual(expected: "Invalid JSON HTTP response from Gateway.", actual: documentClientException.Error.Message); + } + + /// + /// Testing the exception behavior when a response from the Gateway has a response (deserializable Error object) based the content length. + /// For more information, see . + /// + /// + [TestMethod] + [DataRow(@"This is the content of a test error message.")] + public async Task CreateDocumentClientExceptionValidJsonResponseFromGatewayTestAsync(string content) + { + HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + { + RequestMessage = new HttpRequestMessage( + method: HttpMethod.Get, + requestUri: @"https://pt_ac_test_uri.com/"), + Content = new StringContent( + content: JsonConvert.SerializeObject( + value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = content })), + }; + + responseMessage.Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/json"); + + IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( + startTime: DateTime.UtcNow, + trace: NoOpTrace.Singleton); + + DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( + responseMessage: responseMessage, + requestStatistics: requestStatistics); + + Assert.IsNotNull(value: documentClientException); + Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); + Assert.IsTrue(condition: documentClientException.Message.Contains(content)); + + Assert.IsNotNull(value: documentClientException.Error); + Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); + Assert.AreEqual(expected: content, actual: documentClientException.Error.Message); + } + } +} From bee917ff59f2536216125280fc8d0ca1884028fe Mon Sep 17 00:00:00 2001 From: philipthomas Date: Thu, 28 Dec 2023 09:23:33 -0500 Subject: [PATCH 02/15] add back a using to fix build --- Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index e74ef4be59..9975c4a3c3 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -170,7 +170,7 @@ internal static async Task CreateDocumentClientExceptio } Stream readStream = await responseMessage.Content.ReadAsStreamAsync(); - Error error = Resource.LoadFrom(readStream); + Error error = Documents.Resource.LoadFrom(readStream); return new DocumentClientException( error, responseMessage.Headers, From b4ab7f58a703e69ac7d73d2dae35586fe9cdea55 Mon Sep 17 00:00:00 2001 From: Philip Thomas Date: Tue, 2 Jan 2024 18:26:11 -0500 Subject: [PATCH 03/15] change exception message --- Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs | 2 +- .../Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index 9975c4a3c3..bbded98dc9 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -158,7 +158,7 @@ internal static async Task CreateDocumentClientExceptio new Error { Code = responseMessage.StatusCode.ToString(), - Message = "Invalid JSON HTTP response from Gateway." + Message = "No response content from gateway." }, responseMessage.Headers, responseMessage.StatusCode) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs index 581d4ed0d7..f6504395d4 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs @@ -50,11 +50,11 @@ public async Task CreateDocumentClientExceptionInvalidJsonResponseFromGatewayTes Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); - Assert.IsTrue(condition: documentClientException.Message.Contains("Invalid JSON HTTP response from Gateway.")); + Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway.")); Assert.IsNotNull(value: documentClientException.Error); Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); - Assert.AreEqual(expected: "Invalid JSON HTTP response from Gateway.", actual: documentClientException.Error.Message); + Assert.AreEqual(expected: "No response content from gateway.", actual: documentClientException.Error.Message); } /// From 8992c0beae243abe16c3a15d0437ec65d02a6e32 Mon Sep 17 00:00:00 2001 From: Philip Thomas Date: Wed, 3 Jan 2024 02:47:28 -0500 Subject: [PATCH 04/15] check for json validity using parse --- .../src/GatewayStoreClient.cs | 38 +++++++++++-------- .../GatewayStoreClientTests.cs | 8 ++++ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index bbded98dc9..8622e2b5df 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -20,6 +20,7 @@ namespace Microsoft.Azure.Cosmos using Microsoft.Azure.Documents; using Microsoft.Azure.Documents.Collections; using Newtonsoft.Json; + using Newtonsoft.Json.Linq; internal class GatewayStoreClient : TransportClient { @@ -152,25 +153,22 @@ internal static async Task CreateDocumentClientExceptio if (string.Equals(responseMessage.Content?.Headers?.ContentType?.MediaType, "application/json", StringComparison.OrdinalIgnoreCase)) { // For more information, see https://github.com/Azure/azure-cosmos-dotnet-v3/issues/4162. + Error error; + if (await GatewayStoreClient.IsJsonHTTPResponseFromGatewayInvalidAsync(responseMessage)) { - return new DocumentClientException( - new Error - { - Code = responseMessage.StatusCode.ToString(), - Message = "No response content from gateway." - }, - responseMessage.Headers, - responseMessage.StatusCode) + error = new Error { - StatusDescription = responseMessage.ReasonPhrase, - ResourceAddress = resourceIdOrFullName, - RequestStatistics = requestStatistics + Code = responseMessage.StatusCode.ToString(), + Message = "No response content from gateway." }; } + else + { + Stream readStream = await responseMessage.Content.ReadAsStreamAsync(); + error = Documents.Resource.LoadFrom(readStream); + } - Stream readStream = await responseMessage.Content.ReadAsStreamAsync(); - Error error = Documents.Resource.LoadFrom(readStream); return new DocumentClientException( error, responseMessage.Headers, @@ -225,8 +223,18 @@ private static async Task IsJsonHTTPResponseFromGatewayInvalidAsync(HttpRe { string readString = await responseMessage.Content.ReadAsStringAsync(); - return responseMessage.Content?.Headers?.ContentLength == 0 || - readString.Trim().Length == 0; + try + { + _ = JToken.Parse(readString); + + return responseMessage.Content?.Headers?.ContentLength == 0 || + readString.Trim().Length == 0; + } + catch (JsonReaderException) + { + return true; + } + } internal static bool IsAllowedRequestHeader(string headerName) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs index f6504395d4..2dbdb83e31 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs @@ -27,6 +27,14 @@ public class GatewayStoreClientTests [TestMethod] [DataRow(@"")] [DataRow(@" ")] + [DataRow(@"")] + [DataRow(@" ")] + [DataRow(@" ")] + [DataRow(@" ")] + [DataRow(@"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")] + [DataRow(@" ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")] + [DataRow(@"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890 ")] + [DataRow(@" ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890 ")] public async Task CreateDocumentClientExceptionInvalidJsonResponseFromGatewayTestAsync(string content) { HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) From 4886080a80fe1eb6d42a3e537419b03b14501b82 Mon Sep 17 00:00:00 2001 From: philipthomas Date: Wed, 3 Jan 2024 17:26:31 -0500 Subject: [PATCH 05/15] some refactoring and tests. I want to see if I can mine more tests later. --- .../src/GatewayStoreClient.cs | 83 +++++---- .../GatewayStoreClientTests.cs | 172 +++++++++++++++--- 2 files changed, 187 insertions(+), 68 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index 8622e2b5df..e0eb71f12d 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -134,28 +134,56 @@ internal static INameValueCollection ExtractResponseHeaders(HttpResponseMessage return headers; } + /// + /// Creating a new DocumentClientException using the Gateway response message. + /// Is the media type not "application/json"? + /// return DocumentClientExcpetion with responseMessage and header information. + /// + /// Is the header content-length == 0 and media type is "application/json"? Test case sensitivity. + /// return DocumentClientException with message 'No response content from gateway.' + /// + /// Is the content actual length == 0 after a trim and media type is "application/json"? Test case sensitivity. Whitespace scenarios. + /// return DocumentClientException with message 'No response content from gateway.' + /// + /// Is the content not parseable as json, but content length != 0 and media type is "application/json"? Test case sensitivity. + /// return DocumentClientException with message set to raw non-json message from response. + /// + /// + /// internal static async Task CreateDocumentClientExceptionAsync( HttpResponseMessage responseMessage, IClientSideRequestStatistics requestStatistics) { - bool isNameBased = false; - bool isFeed = false; - string resourceTypeString; - string resourceIdOrFullName; + if (responseMessage is null) + { + throw new ArgumentNullException(nameof(responseMessage)); + } + + if (requestStatistics is null) + { + throw new ArgumentNullException(nameof(requestStatistics)); + } + + // Ask, what is the purpose of this, really? + // The only impact of try parse fail is an empty resourceIdOrFullName. - string resourceLink = responseMessage.RequestMessage.RequestUri.LocalPath; - if (!PathsHelper.TryParsePathSegments(resourceLink, out isFeed, out resourceTypeString, out resourceIdOrFullName, out isNameBased)) + if (!PathsHelper.TryParsePathSegments( + resourceUrl: responseMessage.RequestMessage.RequestUri.LocalPath, + isFeed: out _, + resourcePath: out _, + resourceIdOrFullName: out string resourceIdOrFullName, + isNameBased: out _)) { // if resourceLink is invalid - we will not set resourceAddress in exception. } - // If service rejects the initial payload like header is to large it will return an HTML error instead of JSON. - if (string.Equals(responseMessage.Content?.Headers?.ContentType?.MediaType, "application/json", StringComparison.OrdinalIgnoreCase)) + try { - // For more information, see https://github.com/Azure/azure-cosmos-dotnet-v3/issues/4162. - Error error; + Stream readStream = await responseMessage.Content.ReadAsStreamAsync(); + Error error = Documents.Resource.LoadFrom(readStream); - if (await GatewayStoreClient.IsJsonHTTPResponseFromGatewayInvalidAsync(responseMessage)) + if (responseMessage.Content?.Headers?.ContentLength == 0 || + error.Message.Trim().Length == 0) { error = new Error { @@ -163,11 +191,6 @@ internal static async Task CreateDocumentClientExceptio Message = "No response content from gateway." }; } - else - { - Stream readStream = await responseMessage.Content.ReadAsStreamAsync(); - error = Documents.Resource.LoadFrom(readStream); - } return new DocumentClientException( error, @@ -179,7 +202,7 @@ internal static async Task CreateDocumentClientExceptio RequestStatistics = requestStatistics }; } - else + catch { StringBuilder context = new StringBuilder(); context.AppendLine(await responseMessage.Content.ReadAsStringAsync()); @@ -187,7 +210,7 @@ internal static async Task CreateDocumentClientExceptio HttpRequestMessage requestMessage = responseMessage.RequestMessage; if (requestMessage != null) { - context.AppendLine($"RequestUri: {requestMessage.RequestUri.ToString()};"); + context.AppendLine($"RequestUri: {requestMessage.RequestUri};"); context.AppendLine($"RequestMethod: {requestMessage.Method.Method};"); if (requestMessage.Headers != null) @@ -199,7 +222,6 @@ internal static async Task CreateDocumentClientExceptio } } - String message = await responseMessage.Content.ReadAsStringAsync(); return new DocumentClientException( message: context.ToString(), innerException: null, @@ -214,29 +236,6 @@ internal static async Task CreateDocumentClientExceptio } } - /// - /// Checking if exception response (deserializable Error object) is valid based on the content length. - /// For more information, see . - /// - /// - private static async Task IsJsonHTTPResponseFromGatewayInvalidAsync(HttpResponseMessage responseMessage) - { - string readString = await responseMessage.Content.ReadAsStringAsync(); - - try - { - _ = JToken.Parse(readString); - - return responseMessage.Content?.Headers?.ContentLength == 0 || - readString.Trim().Length == 0; - } - catch (JsonReaderException) - { - return true; - } - - } - internal static bool IsAllowedRequestHeader(string headerName) { if (!headerName.StartsWith("x-ms", StringComparison.OrdinalIgnoreCase)) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs index 2dbdb83e31..fc4284ebc1 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs @@ -6,6 +6,8 @@ namespace Microsoft.Azure.Cosmos using System; using System.Net; using System.Net.Http; + using System.Reflection.Metadata; + using System.Text; using System.Threading.Tasks; using Microsoft.Azure.Cosmos.Tracing; using Microsoft.Azure.Cosmos.Tracing.TraceData; @@ -20,13 +22,81 @@ namespace Microsoft.Azure.Cosmos public class GatewayStoreClientTests { /// - /// Testing the exception behavior when a response from the Gateway has no response (deserializable Error object) based on the content length. - /// For more information, see . + /// Testing CreateDocumentClientExceptionAsync when media type is not application/json. Not meant to be an exhaustive test for all + /// legitimate content media types. + /// + [TestMethod] + [DataRow("text/html", "")] + [DataRow("text/plain", "This is a test error message.")] + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplicationJsonAsync( + string mediaType, + string contentMessage) + { + HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + { + RequestMessage = new HttpRequestMessage( + method: HttpMethod.Get, + requestUri: @"https://pt_ac_test_uri.com/"), + Content = new StringContent( + mediaType: mediaType, + encoding: Encoding.UTF8, + content: JsonConvert.SerializeObject( + value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })), + }; + + DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( + responseMessage: responseMessage, + requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); + + Assert.IsNotNull(value: documentClientException); + Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); + Assert.IsTrue(condition: documentClientException.Message.Contains(contentMessage)); + + Assert.IsNotNull(value: documentClientException.Error); + Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); + Assert.IsTrue(documentClientException.Error.Message.Contains(contentMessage)); + } + + /// + /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the header content length is zero. + /// + [TestMethod] + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndHeaderContentLengthIsZeroAsync() + { + HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + { + RequestMessage = new HttpRequestMessage( + method: HttpMethod.Get, + requestUri: @"https://pt_ac_test_uri.com/"), + Content = new StringContent( + mediaType: "application/json", + encoding: Encoding.UTF8, + content: JsonConvert.SerializeObject( + value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = "" })), + }; + + IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( + startTime: DateTime.UtcNow, + trace: NoOpTrace.Singleton); + + DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( + responseMessage: responseMessage, + requestStatistics: requestStatistics); + + Assert.IsNotNull(value: documentClientException); + Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); + Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway.")); + + Assert.IsNotNull(value: documentClientException.Error); + Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); + Assert.AreEqual(expected: "No response content from gateway.", actual: documentClientException.Error.Message); + } + + /// + /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the content is not valid json + /// and has a content length that is not zero after trim. /// - /// [TestMethod] - [DataRow(@"")] - [DataRow(@" ")] [DataRow(@"")] [DataRow(@" ")] [DataRow(@" ")] @@ -35,7 +105,7 @@ public class GatewayStoreClientTests [DataRow(@" ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")] [DataRow(@"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890 ")] [DataRow(@" ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890 ")] - public async Task CreateDocumentClientExceptionInvalidJsonResponseFromGatewayTestAsync(string content) + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndContentIsNotValidJsonAndContentLengthIsNotZeroAsync(string contentMessage) { HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) { @@ -43,36 +113,37 @@ public async Task CreateDocumentClientExceptionInvalidJsonResponseFromGatewayTes method: HttpMethod.Get, requestUri: @"https://pt_ac_test_uri.com/"), Content = new StringContent( - content: content), + mediaType: "application/json", + encoding: Encoding.UTF8, + content: JsonConvert.SerializeObject( + value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })), }; - responseMessage.Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/json"); - IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( - startTime: DateTime.UtcNow, + startTime: DateTime.UtcNow, trace: NoOpTrace.Singleton); DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( - responseMessage: responseMessage, - requestStatistics: requestStatistics); + responseMessage: responseMessage, + requestStatistics: requestStatistics); Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); - Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway.")); + Assert.IsTrue(condition: documentClientException.Message.Contains(contentMessage)); Assert.IsNotNull(value: documentClientException.Error); Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); - Assert.AreEqual(expected: "No response content from gateway.", actual: documentClientException.Error.Message); + Assert.AreEqual(expected: contentMessage, actual: documentClientException.Error.Message); } /// - /// Testing the exception behavior when a response from the Gateway has a response (deserializable Error object) based the content length. - /// For more information, see . + /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the content is not valid json + /// and has a content length that is zero after trim. /// - /// [TestMethod] - [DataRow(@"This is the content of a test error message.")] - public async Task CreateDocumentClientExceptionValidJsonResponseFromGatewayTestAsync(string content) + [DataRow(@"")] + [DataRow(@" ")] + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndContentIsNotValidJsonAndContentLengthIsZeroAsync(string contentMessage) { HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) { @@ -81,26 +152,75 @@ public async Task CreateDocumentClientExceptionValidJsonResponseFromGatewayTestA requestUri: @"https://pt_ac_test_uri.com/"), Content = new StringContent( content: JsonConvert.SerializeObject( - value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = content })), + value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })), }; - responseMessage.Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/json"); - IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( startTime: DateTime.UtcNow, trace: NoOpTrace.Singleton); DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( - responseMessage: responseMessage, - requestStatistics: requestStatistics); + responseMessage: responseMessage, + requestStatistics: requestStatistics); Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); - Assert.IsTrue(condition: documentClientException.Message.Contains(content)); + Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway.")); Assert.IsNotNull(value: documentClientException.Error); Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); - Assert.AreEqual(expected: content, actual: documentClientException.Error.Message); + Assert.AreEqual(expected: "No response content from gateway.", actual: documentClientException.Error.Message); + } + + /// + /// Testing CreateDocumentClientExceptionAsync when response message argument is null, then expects an argumentNullException. + /// + [TestMethod] + public async Task TestCreateDocumentClientExceptionWhenResponseMessageIsNullExpectsArgumentNullException() + { + IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( + startTime: DateTime.UtcNow, + trace: NoOpTrace.Singleton); + + ArgumentNullException argumentNullException = await Assert.ThrowsExceptionAsync(async () => await GatewayStoreClient.CreateDocumentClientExceptionAsync( + responseMessage: default, + requestStatistics: requestStatistics) + ); + + Assert.IsNotNull(argumentNullException); + Assert.AreEqual(expected: "Value cannot be null. (Parameter 'responseMessage')", actual: argumentNullException.Message); + } + + /// + /// Testing CreateDocumentClientExceptionAsync when request statistics argument is null, then expects an argumentNullException. + /// + [TestMethod] + public async Task TestCreateDocumentClientExceptionWhenRequestStatisticsIsNullExpectsArgumentNullException() + { + HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + { + RequestMessage = new HttpRequestMessage( + method: HttpMethod.Get, + requestUri: @"https://pt_ac_test_uri.com/"), + Content = new StringContent( + content: JsonConvert.SerializeObject( + value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = "" })), + }; + + ArgumentNullException argumentNullException = await Assert.ThrowsExceptionAsync(async () => await GatewayStoreClient.CreateDocumentClientExceptionAsync( + responseMessage: responseMessage, + requestStatistics: default) + ); + + Assert.IsNotNull(argumentNullException); + Assert.AreEqual(expected: "Value cannot be null. (Parameter 'requestStatistics')", actual: argumentNullException.Message); + } + + private static IClientSideRequestStatistics CreateClientSideRequestStatistics() + { + return new ClientSideRequestStatisticsTraceDatum( + startTime: DateTime.UtcNow, + trace: NoOpTrace.Singleton); } } } From 2f582f648cd240ba73ce6834c1603400f5c7e975 Mon Sep 17 00:00:00 2001 From: philipthomas Date: Wed, 3 Jan 2024 17:30:46 -0500 Subject: [PATCH 06/15] comment to rethink removing the media type check --- Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index e0eb71f12d..f2a3cbf58f 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -182,6 +182,8 @@ internal static async Task CreateDocumentClientExceptio Stream readStream = await responseMessage.Content.ReadAsStreamAsync(); Error error = Documents.Resource.LoadFrom(readStream); + // need to rethink dropping the check for media type "application/json". + if (responseMessage.Content?.Headers?.ContentLength == 0 || error.Message.Trim().Length == 0) { From dd71297069480c3abd163d30a4f1e792cabc71b0 Mon Sep 17 00:00:00 2001 From: philipthomas Date: Thu, 4 Jan 2024 13:12:29 -0500 Subject: [PATCH 07/15] more tests and refactoring. going to mine out more tests before I undraft it --- .../src/GatewayStoreClient.cs | 180 +++++++++++++++--- .../GatewayStoreClientTests.cs | 51 ++++- 2 files changed, 203 insertions(+), 28 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index f2a3cbf58f..57222d01dc 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -10,6 +10,8 @@ namespace Microsoft.Azure.Cosmos using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; + using System.Linq.Expressions; + using System.Net; using System.Net.Http; using System.Net.Http.Headers; using System.Text; @@ -136,17 +138,6 @@ internal static INameValueCollection ExtractResponseHeaders(HttpResponseMessage /// /// Creating a new DocumentClientException using the Gateway response message. - /// Is the media type not "application/json"? - /// return DocumentClientExcpetion with responseMessage and header information. - /// - /// Is the header content-length == 0 and media type is "application/json"? Test case sensitivity. - /// return DocumentClientException with message 'No response content from gateway.' - /// - /// Is the content actual length == 0 after a trim and media type is "application/json"? Test case sensitivity. Whitespace scenarios. - /// return DocumentClientException with message 'No response content from gateway.' - /// - /// Is the content not parseable as json, but content length != 0 and media type is "application/json"? Test case sensitivity. - /// return DocumentClientException with message set to raw non-json message from response. /// /// /// @@ -164,6 +155,134 @@ internal static async Task CreateDocumentClientExceptio throw new ArgumentNullException(nameof(requestStatistics)); } + if (!PathsHelper.TryParsePathSegments( + resourceUrl: responseMessage.RequestMessage.RequestUri.LocalPath, + isFeed: out _, + resourcePath: out _, + resourceIdOrFullName: out string resourceIdOrFullName, + isNameBased: out _)) + { + // if resourceLink is invalid - we will not set resourceAddress in exception. + } + + try + { + Stream contentAsStream = await responseMessage.Content.ReadAsStreamAsync(); + Error error = GatewayStoreClient.GetError( + error: Documents.Resource.LoadFrom(stream: contentAsStream), + statusCode: responseMessage.StatusCode); + + return new DocumentClientException( + errorResource: error, + responseHeaders: responseMessage.Headers, + statusCode: responseMessage.StatusCode) + { + StatusDescription = responseMessage.ReasonPhrase, + ResourceAddress = resourceIdOrFullName, + RequestStatistics = requestStatistics + }; + } + catch + { + StringBuilder contextBuilder = new StringBuilder( + value: GatewayStoreClient.GetError( + error: await responseMessage.Content.ReadAsStringAsync(), + statusCode: responseMessage.StatusCode)); + + HttpRequestMessage requestMessage = responseMessage.RequestMessage; + + if (requestMessage != null) + { + contextBuilder.AppendLine($"RequestUri: {requestMessage.RequestUri};"); + contextBuilder.AppendLine($"RequestMethod: {requestMessage.Method.Method};"); + + if (requestMessage.Headers != null) + { + foreach (KeyValuePair> header in requestMessage.Headers) + { + contextBuilder.AppendLine($"Header: {header.Key} Length: {string.Join(",", header.Value).Length};"); + } + } + } + + return new DocumentClientException( + message: contextBuilder.ToString(), + innerException: null, + responseHeaders: responseMessage.Headers, + statusCode: responseMessage.StatusCode, + requestUri: responseMessage.RequestMessage.RequestUri) + { + StatusDescription = responseMessage.ReasonPhrase, + ResourceAddress = resourceIdOrFullName, + RequestStatistics = requestStatistics + }; + } + } + + private static readonly string NORESPONSECONTENTFROMGATEWAY = "No response content from gateway."; + + /// + /// Get or create an Error type using an existing Error type. + /// + /// + /// + private static Error GetError( + Error error, + HttpStatusCode statusCode) + { + if (error.Message.Trim().Length == 0) + { + return new Error + { + Code = statusCode.ToString(), + Message = GatewayStoreClient.NORESPONSECONTENTFROMGATEWAY, + }; + } + + return error; + } + + /// + /// Get or set an Error string using an existing Error string. + /// + /// + /// + private static string GetError( + string error, + HttpStatusCode statusCode) + { + if (error.Trim().Length == 0) + { + return JsonConvert.SerializeObject( + new Error + { + Code = statusCode.ToString(), + Message = GatewayStoreClient.NORESPONSECONTENTFROMGATEWAY, + }); + } + + return error; + } + + /// + /// Creating a new DocumentClientException using the Gateway response message. + /// + /// + /// + internal static async Task WorkingCreateDocumentClientExceptionAsync( + HttpResponseMessage responseMessage, + IClientSideRequestStatistics requestStatistics) + { + if (responseMessage is null) + { + throw new ArgumentNullException(nameof(responseMessage)); + } + + if (requestStatistics is null) + { + throw new ArgumentNullException(nameof(requestStatistics)); + } + // Ask, what is the purpose of this, really? // The only impact of try parse fail is an empty resourceIdOrFullName. @@ -177,21 +296,36 @@ internal static async Task CreateDocumentClientExceptio // if resourceLink is invalid - we will not set resourceAddress in exception. } + Error error; + try - { - Stream readStream = await responseMessage.Content.ReadAsStreamAsync(); - Error error = Documents.Resource.LoadFrom(readStream); + { + await Console.Out.WriteLineAsync($"ContentLength: {responseMessage.Content?.Headers?.ContentLength}"); + + if (!string.Equals(responseMessage.Content?.Headers?.ContentType?.MediaType, "application/json", StringComparison.OrdinalIgnoreCase)) + { + throw new ArgumentNullException(); + } + + // Check ContentLength on Header and Length of trimmed Message's Content. - // need to rethink dropping the check for media type "application/json". + string messageContent = await responseMessage.Content.ReadAsStringAsync(); + int messageContentLength = messageContent.Trim().Length; + long? headerContentLength = responseMessage.Content?.Headers?.ContentLength; - if (responseMessage.Content?.Headers?.ContentLength == 0 || - error.Message.Trim().Length == 0) + if (headerContentLength == 0 || messageContentLength == 0) { - error = new Error + error = new Error { Code = responseMessage.StatusCode.ToString(), Message = "No response content from gateway." }; + } + else + { + Stream readStream = await responseMessage.Content.ReadAsStreamAsync(); + error = Documents.Resource.LoadFrom(readStream); + + if (error.Message.Trim().Length == 0) { - Code = responseMessage.StatusCode.ToString(), - Message = "No response content from gateway." - }; + error.Message = "No response content from gateway."; + } } return new DocumentClientException( @@ -207,7 +341,9 @@ internal static async Task CreateDocumentClientExceptio catch { StringBuilder context = new StringBuilder(); - context.AppendLine(await responseMessage.Content.ReadAsStringAsync()); + string readString = await responseMessage.Content.ReadAsStringAsync(); + + context.AppendLine(readString); HttpRequestMessage requestMessage = responseMessage.RequestMessage; if (requestMessage != null) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs index fc4284ebc1..977fc8c6c0 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs @@ -58,10 +58,48 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplication } /// - /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the header content length is zero. + /// Testing CreateDocumentClientExceptionAsync when media type is not application/json. Not meant to be an exhaustive test for all + /// legitimate content media types. + /// + [TestMethod] + [DataRow("text/html", "")] + [DataRow("text/html", " ")] + [DataRow("text/plain", "")] + [DataRow("text/plain", " ")] + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplicationJsonAndHeaderContentLengthIsZeroAsync( + string mediaType, + string contentMessage) + { + HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + { + RequestMessage = new HttpRequestMessage( + method: HttpMethod.Get, + requestUri: @"https://pt_ac_test_uri.com/"), + Content = new StringContent( + mediaType: mediaType, + encoding: Encoding.UTF8, + content: JsonConvert.SerializeObject( + value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })), + }; + + DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( + responseMessage: responseMessage, + requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); + + Assert.IsNotNull(value: documentClientException); + Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); + Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway.")); + + Assert.IsNotNull(value: documentClientException.Error); + Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); + Assert.IsTrue(documentClientException.Error.Message.Contains("No response content from gateway.")); + } + + /// + /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the error message length is zero. /// [TestMethod] - public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndHeaderContentLengthIsZeroAsync() + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndErrorMessageLengthIsZeroAsync() { HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) { @@ -143,7 +181,7 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso [TestMethod] [DataRow(@"")] [DataRow(@" ")] - public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndContentIsNotValidJsonAndContentLengthIsZeroAsync(string contentMessage) + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndContentIsNotValidJsonAndHeaderContentLengthIsZeroAsync(string contentMessage) { HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) { @@ -151,8 +189,9 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso method: HttpMethod.Get, requestUri: @"https://pt_ac_test_uri.com/"), Content = new StringContent( - content: JsonConvert.SerializeObject( - value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })), + mediaType: "application/json", + encoding: Encoding.UTF8, + content: contentMessage), }; IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( @@ -169,7 +208,7 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso Assert.IsNotNull(value: documentClientException.Error); Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); - Assert.AreEqual(expected: "No response content from gateway.", actual: documentClientException.Error.Message); + Assert.IsTrue(documentClientException.Error.Message.Contains("No response content from gateway.")); } /// From 4d0fa3cc829391dc86b1c9d72abb9be742c0013c Mon Sep 17 00:00:00 2001 From: philipthomas Date: Thu, 4 Jan 2024 13:15:31 -0500 Subject: [PATCH 08/15] moved string to top of class --- Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index 57222d01dc..9508f54084 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -30,6 +30,7 @@ internal class GatewayStoreClient : TransportClient private readonly CosmosHttpClient httpClient; private readonly JsonSerializerSettings SerializerSettings; private static readonly HttpMethod httpPatchMethod = new HttpMethod(HttpConstants.HttpMethods.Patch); + private static readonly string NoResponseContentFromGateway = "No response content from gateway."; public GatewayStoreClient( CosmosHttpClient httpClient, @@ -219,8 +220,6 @@ internal static async Task CreateDocumentClientExceptio } } - private static readonly string NORESPONSECONTENTFROMGATEWAY = "No response content from gateway."; - /// /// Get or create an Error type using an existing Error type. /// @@ -235,7 +234,7 @@ private static Error GetError( return new Error { Code = statusCode.ToString(), - Message = GatewayStoreClient.NORESPONSECONTENTFROMGATEWAY, + Message = GatewayStoreClient.NoResponseContentFromGateway, }; } @@ -257,7 +256,7 @@ private static string GetError( new Error { Code = statusCode.ToString(), - Message = GatewayStoreClient.NORESPONSECONTENTFROMGATEWAY, + Message = GatewayStoreClient.NoResponseContentFromGateway, }); } From 573e2ed43af7210cbd123cf3457073b5442e6269 Mon Sep 17 00:00:00 2001 From: philipthomas Date: Thu, 4 Jan 2024 15:01:59 -0500 Subject: [PATCH 09/15] more tests and some refactoring --- .../src/GatewayStoreClient.cs | 4 +- .../GatewayStoreClientTests.cs | 134 ++++++++++++------ 2 files changed, 95 insertions(+), 43 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index 9508f54084..9279996e94 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -185,8 +185,8 @@ internal static async Task CreateDocumentClientExceptio } catch { - StringBuilder contextBuilder = new StringBuilder( - value: GatewayStoreClient.GetError( + StringBuilder contextBuilder = new StringBuilder(); + contextBuilder.AppendLine(GatewayStoreClient.GetError( error: await responseMessage.Content.ReadAsStringAsync(), statusCode: responseMessage.StatusCode)); diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs index 977fc8c6c0..a2bc1d6666 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs @@ -22,15 +22,17 @@ namespace Microsoft.Azure.Cosmos public class GatewayStoreClientTests { /// - /// Testing CreateDocumentClientExceptionAsync when media type is not application/json. Not meant to be an exhaustive test for all - /// legitimate content media types. + /// Testing CreateDocumentClientExceptionAsync when media type is NOT application/json and the error message has a length that is not zero. + /// This is not meant to be an exhaustive test for all legitimate content media types. + /// /// [TestMethod] [DataRow("text/html", "")] [DataRow("text/plain", "This is a test error message.")] - public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplicationJsonAsync( + [Owner("philipthomas-MSFT")] + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplicationJsonAndErrorMessageLengthIsNotZeroAsync( string mediaType, - string contentMessage) + string errorMessage) { HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) { @@ -41,7 +43,7 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplication mediaType: mediaType, encoding: Encoding.UTF8, content: JsonConvert.SerializeObject( - value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })), + value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = errorMessage })), }; DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( @@ -50,25 +52,27 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplication Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); - Assert.IsTrue(condition: documentClientException.Message.Contains(contentMessage)); + Assert.IsTrue(condition: documentClientException.Message.Contains(errorMessage)); Assert.IsNotNull(value: documentClientException.Error); Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); - Assert.IsTrue(documentClientException.Error.Message.Contains(contentMessage)); + Assert.IsTrue(documentClientException.Error.Message.Contains(errorMessage)); } /// - /// Testing CreateDocumentClientExceptionAsync when media type is not application/json. Not meant to be an exhaustive test for all - /// legitimate content media types. + /// Testing CreateDocumentClientExceptionAsync when media type is NOT application/json and the error message has a length that is zero. + /// This is not meant to be an exhaustive test for all legitimate content media types. + /// /// [TestMethod] [DataRow("text/html", "")] [DataRow("text/html", " ")] [DataRow("text/plain", "")] [DataRow("text/plain", " ")] - public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplicationJsonAndHeaderContentLengthIsZeroAsync( + [Owner("philipthomas-MSFT")] + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplicationJsonAndErrorMessageLengthIsZeroAsync( string mediaType, - string contentMessage) + string errorMessage) { HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) { @@ -79,7 +83,7 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplication mediaType: mediaType, encoding: Encoding.UTF8, content: JsonConvert.SerializeObject( - value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })), + value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = errorMessage })), }; DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( @@ -95,11 +99,58 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplication Assert.IsTrue(documentClientException.Error.Message.Contains("No response content from gateway.")); } + /// + /// Testing CreateDocumentClientExceptionAsync when media type is NOT application/json and the header content length is zero. + /// This is not meant to be an exhaustive test for all legitimate content media types. + /// + /// + [TestMethod] + [DataRow("text/plain", @"")] + [DataRow("text/plain", @" ")] + [Owner("philipthomas-MSFT")] + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplicationJsonAndHeaderContentLengthIsZeroAsync( + string mediaType, + string contentMessage) + { + HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + { + RequestMessage = new HttpRequestMessage( + method: HttpMethod.Get, + requestUri: @"https://pt_ac_test_uri.com/"), + Content = new StringContent( + mediaType: mediaType, + encoding: Encoding.UTF8, + content: contentMessage), + }; + + IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( + startTime: DateTime.UtcNow, + trace: NoOpTrace.Singleton); + + DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( + responseMessage: responseMessage, + requestStatistics: requestStatistics); + + Assert.IsNotNull(value: documentClientException); + Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); + Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway.")); + + Assert.IsNotNull(value: documentClientException.Error); + Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); + Assert.IsTrue(documentClientException.Error.Message.Contains("No response content from gateway.")); + } + /// /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the error message length is zero. + /// /// [TestMethod] - public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndErrorMessageLengthIsZeroAsync() + [DataRow("application/json", "")] + [DataRow("application/json", " ")] + [Owner("philipthomas-MSFT")] + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndErrorMessageLengthIsZeroAsync( + string mediaType, + string errorMessage) { HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) { @@ -107,10 +158,10 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso method: HttpMethod.Get, requestUri: @"https://pt_ac_test_uri.com/"), Content = new StringContent( - mediaType: "application/json", + mediaType: mediaType, encoding: Encoding.UTF8, content: JsonConvert.SerializeObject( - value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = "" })), + value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = errorMessage })), }; IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( @@ -131,19 +182,23 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso } /// - /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the content is not valid json + /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the content message is not valid json. /// and has a content length that is not zero after trim. + /// /// [TestMethod] - [DataRow(@"")] - [DataRow(@" ")] - [DataRow(@" ")] - [DataRow(@" ")] - [DataRow(@"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")] - [DataRow(@" ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")] - [DataRow(@"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890 ")] - [DataRow(@" ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890 ")] - public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndContentIsNotValidJsonAndContentLengthIsNotZeroAsync(string contentMessage) + [DataRow("application/json", @"")] + [DataRow("application/json", @" ")] + [DataRow("application/json", @" ")] + [DataRow("application/json", @" ")] + [DataRow("application/json", @"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")] + [DataRow("application/json", @" ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")] + [DataRow("application/json", @"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890 ")] + [DataRow("application/json", @" ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890 ")] + [Owner("philipthomas-MSFT")] + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndContentMessageIsNotValidJsonAsync( + string mediaType, + string contentMessage) { HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) { @@ -151,10 +206,9 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso method: HttpMethod.Get, requestUri: @"https://pt_ac_test_uri.com/"), Content = new StringContent( - mediaType: "application/json", + mediaType: mediaType, encoding: Encoding.UTF8, - content: JsonConvert.SerializeObject( - value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })), + content: contentMessage), }; IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( @@ -168,20 +222,18 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); Assert.IsTrue(condition: documentClientException.Message.Contains(contentMessage)); - - Assert.IsNotNull(value: documentClientException.Error); - Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); - Assert.AreEqual(expected: contentMessage, actual: documentClientException.Error.Message); } /// - /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the content is not valid json - /// and has a content length that is zero after trim. + /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the header content length is zero. /// [TestMethod] - [DataRow(@"")] - [DataRow(@" ")] - public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndContentIsNotValidJsonAndHeaderContentLengthIsZeroAsync(string contentMessage) + [DataRow("application/json", @"")] + [DataRow("application/json", @" ")] + [Owner("philipthomas-MSFT")] + public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndHeaderContentLengthIsZeroAsync( + string mediaType, + string contentMessage) { HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) { @@ -189,7 +241,7 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso method: HttpMethod.Get, requestUri: @"https://pt_ac_test_uri.com/"), Content = new StringContent( - mediaType: "application/json", + mediaType: mediaType, encoding: Encoding.UTF8, content: contentMessage), }; @@ -215,11 +267,10 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso /// Testing CreateDocumentClientExceptionAsync when response message argument is null, then expects an argumentNullException. /// [TestMethod] + [Owner("philipthomas-MSFT")] public async Task TestCreateDocumentClientExceptionWhenResponseMessageIsNullExpectsArgumentNullException() { - IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( - startTime: DateTime.UtcNow, - trace: NoOpTrace.Singleton); + IClientSideRequestStatistics requestStatistics = GatewayStoreClientTests.CreateClientSideRequestStatistics(); ArgumentNullException argumentNullException = await Assert.ThrowsExceptionAsync(async () => await GatewayStoreClient.CreateDocumentClientExceptionAsync( responseMessage: default, @@ -234,6 +285,7 @@ public async Task TestCreateDocumentClientExceptionWhenResponseMessageIsNullExpe /// Testing CreateDocumentClientExceptionAsync when request statistics argument is null, then expects an argumentNullException. /// [TestMethod] + [Owner("philipthomas-MSFT")] public async Task TestCreateDocumentClientExceptionWhenRequestStatisticsIsNullExpectsArgumentNullException() { HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) From 74f59afa78d72141624fc24b2768846043a7ae1a Mon Sep 17 00:00:00 2001 From: philipthomas Date: Thu, 4 Jan 2024 17:52:50 -0500 Subject: [PATCH 10/15] simplified some more things in tests --- .../GatewayStoreClientTests.cs | 30 ++++--------------- 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs index a2bc1d6666..dd487d625c 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs @@ -123,13 +123,9 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplication content: contentMessage), }; - IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( - startTime: DateTime.UtcNow, - trace: NoOpTrace.Singleton); - DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( responseMessage: responseMessage, - requestStatistics: requestStatistics); + requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); @@ -164,13 +160,9 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = errorMessage })), }; - IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( - startTime: DateTime.UtcNow, - trace: NoOpTrace.Singleton); - DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( responseMessage: responseMessage, - requestStatistics: requestStatistics); + requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); @@ -210,14 +202,10 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso encoding: Encoding.UTF8, content: contentMessage), }; - - IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( - startTime: DateTime.UtcNow, - trace: NoOpTrace.Singleton); - + DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( responseMessage: responseMessage, - requestStatistics: requestStatistics); + requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); @@ -246,13 +234,9 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso content: contentMessage), }; - IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum( - startTime: DateTime.UtcNow, - trace: NoOpTrace.Singleton); - DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( responseMessage: responseMessage, - requestStatistics: requestStatistics); + requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); @@ -270,11 +254,9 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso [Owner("philipthomas-MSFT")] public async Task TestCreateDocumentClientExceptionWhenResponseMessageIsNullExpectsArgumentNullException() { - IClientSideRequestStatistics requestStatistics = GatewayStoreClientTests.CreateClientSideRequestStatistics(); - ArgumentNullException argumentNullException = await Assert.ThrowsExceptionAsync(async () => await GatewayStoreClient.CreateDocumentClientExceptionAsync( responseMessage: default, - requestStatistics: requestStatistics) + requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()) ); Assert.IsNotNull(argumentNullException); From aaaa879853c369f95b44567a9adeb3737dcb4719 Mon Sep 17 00:00:00 2001 From: philipthomas Date: Mon, 8 Jan 2024 05:30:19 -0500 Subject: [PATCH 11/15] removing this method --- .../src/GatewayStoreClient.cs | 110 ------------------ 1 file changed, 110 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index 9279996e94..9d89cb2953 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -263,116 +263,6 @@ private static string GetError( return error; } - /// - /// Creating a new DocumentClientException using the Gateway response message. - /// - /// - /// - internal static async Task WorkingCreateDocumentClientExceptionAsync( - HttpResponseMessage responseMessage, - IClientSideRequestStatistics requestStatistics) - { - if (responseMessage is null) - { - throw new ArgumentNullException(nameof(responseMessage)); - } - - if (requestStatistics is null) - { - throw new ArgumentNullException(nameof(requestStatistics)); - } - - // Ask, what is the purpose of this, really? - // The only impact of try parse fail is an empty resourceIdOrFullName. - - if (!PathsHelper.TryParsePathSegments( - resourceUrl: responseMessage.RequestMessage.RequestUri.LocalPath, - isFeed: out _, - resourcePath: out _, - resourceIdOrFullName: out string resourceIdOrFullName, - isNameBased: out _)) - { - // if resourceLink is invalid - we will not set resourceAddress in exception. - } - - Error error; - - try - { - await Console.Out.WriteLineAsync($"ContentLength: {responseMessage.Content?.Headers?.ContentLength}"); - - if (!string.Equals(responseMessage.Content?.Headers?.ContentType?.MediaType, "application/json", StringComparison.OrdinalIgnoreCase)) - { - throw new ArgumentNullException(); - } - - // Check ContentLength on Header and Length of trimmed Message's Content. - - string messageContent = await responseMessage.Content.ReadAsStringAsync(); - int messageContentLength = messageContent.Trim().Length; - long? headerContentLength = responseMessage.Content?.Headers?.ContentLength; - - if (headerContentLength == 0 || messageContentLength == 0) - { - error = new Error { Code = responseMessage.StatusCode.ToString(), Message = "No response content from gateway." }; - } - else - { - Stream readStream = await responseMessage.Content.ReadAsStreamAsync(); - error = Documents.Resource.LoadFrom(readStream); - - if (error.Message.Trim().Length == 0) - { - error.Message = "No response content from gateway."; - } - } - - return new DocumentClientException( - error, - responseMessage.Headers, - responseMessage.StatusCode) - { - StatusDescription = responseMessage.ReasonPhrase, - ResourceAddress = resourceIdOrFullName, - RequestStatistics = requestStatistics - }; - } - catch - { - StringBuilder context = new StringBuilder(); - string readString = await responseMessage.Content.ReadAsStringAsync(); - - context.AppendLine(readString); - - HttpRequestMessage requestMessage = responseMessage.RequestMessage; - if (requestMessage != null) - { - context.AppendLine($"RequestUri: {requestMessage.RequestUri};"); - context.AppendLine($"RequestMethod: {requestMessage.Method.Method};"); - - if (requestMessage.Headers != null) - { - foreach (KeyValuePair> header in requestMessage.Headers) - { - context.AppendLine($"Header: {header.Key} Length: {string.Join(",", header.Value).Length};"); - } - } - } - - return new DocumentClientException( - message: context.ToString(), - innerException: null, - responseHeaders: responseMessage.Headers, - statusCode: responseMessage.StatusCode, - requestUri: responseMessage.RequestMessage.RequestUri) - { - StatusDescription = responseMessage.ReasonPhrase, - ResourceAddress = resourceIdOrFullName, - RequestStatistics = requestStatistics - }; - } - } - internal static bool IsAllowedRequestHeader(string headerName) { if (!headerName.StartsWith("x-ms", StringComparison.OrdinalIgnoreCase)) From fec474f5122ab513e8348710566f974efb77dfea Mon Sep 17 00:00:00 2001 From: philipthomas Date: Mon, 8 Jan 2024 06:18:47 -0500 Subject: [PATCH 12/15] removed unnecessary usings --- Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index 9d89cb2953..1d1d7f8d73 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -9,20 +9,16 @@ namespace Microsoft.Azure.Cosmos using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.IO; - using System.Linq; - using System.Linq.Expressions; using System.Net; using System.Net.Http; using System.Net.Http.Headers; using System.Text; using System.Threading; using System.Threading.Tasks; - using Microsoft.Azure.Cosmos.Handlers; using Microsoft.Azure.Cosmos.Tracing.TraceData; using Microsoft.Azure.Documents; using Microsoft.Azure.Documents.Collections; using Newtonsoft.Json; - using Newtonsoft.Json.Linq; internal class GatewayStoreClient : TransportClient { From f6f6770a3734b02eea95c03dba67d85c8a21eb00 Mon Sep 17 00:00:00 2001 From: philipthomas Date: Mon, 8 Jan 2024 10:44:26 -0500 Subject: [PATCH 13/15] last refactoring to ignore dealing with Error.Message when empty --- .../src/GatewayStoreClient.cs | 123 ++++++------------ .../GatewayStoreClientTests.cs | 51 ++++---- 2 files changed, 65 insertions(+), 109 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index 1d1d7f8d73..35ca1dcd28 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -9,7 +9,6 @@ namespace Microsoft.Azure.Cosmos using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.IO; - using System.Net; using System.Net.Http; using System.Net.Http.Headers; using System.Text; @@ -26,7 +25,6 @@ internal class GatewayStoreClient : TransportClient private readonly CosmosHttpClient httpClient; private readonly JsonSerializerSettings SerializerSettings; private static readonly HttpMethod httpPatchMethod = new HttpMethod(HttpConstants.HttpMethods.Patch); - private static readonly string NoResponseContentFromGateway = "No response content from gateway."; public GatewayStoreClient( CosmosHttpClient httpClient, @@ -162,101 +160,60 @@ internal static async Task CreateDocumentClientExceptio // if resourceLink is invalid - we will not set resourceAddress in exception. } - try + // If service rejects the initial payload like header is to large it will return an HTML error instead of JSON. + if (string.Equals(responseMessage.Content?.Headers?.ContentType?.MediaType, "application/json", StringComparison.OrdinalIgnoreCase) || + responseMessage.Content?.Headers.ContentLength != 0) { - Stream contentAsStream = await responseMessage.Content.ReadAsStreamAsync(); - Error error = GatewayStoreClient.GetError( - error: Documents.Resource.LoadFrom(stream: contentAsStream), - statusCode: responseMessage.StatusCode); - - return new DocumentClientException( - errorResource: error, - responseHeaders: responseMessage.Headers, - statusCode: responseMessage.StatusCode) + try { - StatusDescription = responseMessage.ReasonPhrase, - ResourceAddress = resourceIdOrFullName, - RequestStatistics = requestStatistics - }; - } - catch - { - StringBuilder contextBuilder = new StringBuilder(); - contextBuilder.AppendLine(GatewayStoreClient.GetError( - error: await responseMessage.Content.ReadAsStringAsync(), - statusCode: responseMessage.StatusCode)); + Stream contentAsStream = await responseMessage.Content.ReadAsStreamAsync(); + Error error = JsonSerializable.LoadFrom(stream: contentAsStream); - HttpRequestMessage requestMessage = responseMessage.RequestMessage; - - if (requestMessage != null) - { - contextBuilder.AppendLine($"RequestUri: {requestMessage.RequestUri};"); - contextBuilder.AppendLine($"RequestMethod: {requestMessage.Method.Method};"); - - if (requestMessage.Headers != null) + return new DocumentClientException( + errorResource: error, + responseHeaders: responseMessage.Headers, + statusCode: responseMessage.StatusCode) { - foreach (KeyValuePair> header in requestMessage.Headers) - { - contextBuilder.AppendLine($"Header: {header.Key} Length: {string.Join(",", header.Value).Length};"); - } - } + StatusDescription = responseMessage.ReasonPhrase, + ResourceAddress = resourceIdOrFullName, + RequestStatistics = requestStatistics + }; } - - return new DocumentClientException( - message: contextBuilder.ToString(), - innerException: null, - responseHeaders: responseMessage.Headers, - statusCode: responseMessage.StatusCode, - requestUri: responseMessage.RequestMessage.RequestUri) + catch { - StatusDescription = responseMessage.ReasonPhrase, - ResourceAddress = resourceIdOrFullName, - RequestStatistics = requestStatistics - }; + } } - } - /// - /// Get or create an Error type using an existing Error type. - /// - /// - /// - private static Error GetError( - Error error, - HttpStatusCode statusCode) - { - if (error.Message.Trim().Length == 0) - { - return new Error - { - Code = statusCode.ToString(), - Message = GatewayStoreClient.NoResponseContentFromGateway, - }; - } + StringBuilder contextBuilder = new StringBuilder(); + contextBuilder.AppendLine(await responseMessage.Content.ReadAsStringAsync()); - return error; - } + HttpRequestMessage requestMessage = responseMessage.RequestMessage; - /// - /// Get or set an Error string using an existing Error string. - /// - /// - /// - private static string GetError( - string error, - HttpStatusCode statusCode) - { - if (error.Trim().Length == 0) + if (requestMessage != null) { - return JsonConvert.SerializeObject( - new Error + contextBuilder.AppendLine($"RequestUri: {requestMessage.RequestUri};"); + contextBuilder.AppendLine($"RequestMethod: {requestMessage.Method.Method};"); + + if (requestMessage.Headers != null) + { + foreach (KeyValuePair> header in requestMessage.Headers) { - Code = statusCode.ToString(), - Message = GatewayStoreClient.NoResponseContentFromGateway, - }); + contextBuilder.AppendLine($"Header: {header.Key} Length: {string.Join(",", header.Value).Length};"); + } + } } - return error; + return new DocumentClientException( + message: contextBuilder.ToString(), + innerException: null, + responseHeaders: responseMessage.Headers, + statusCode: responseMessage.StatusCode, + requestUri: responseMessage.RequestMessage.RequestUri) + { + StatusDescription = responseMessage.ReasonPhrase, + ResourceAddress = resourceIdOrFullName, + RequestStatistics = requestStatistics + }; } internal static bool IsAllowedRequestHeader(string headerName) diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs index dd487d625c..3a537233b7 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs @@ -6,7 +6,6 @@ namespace Microsoft.Azure.Cosmos using System; using System.Net; using System.Net.Http; - using System.Reflection.Metadata; using System.Text; using System.Threading.Tasks; using Microsoft.Azure.Cosmos.Tracing; @@ -34,7 +33,7 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplication string mediaType, string errorMessage) { - HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + HttpResponseMessage responseMessage = new(statusCode: HttpStatusCode.NotFound) { RequestMessage = new HttpRequestMessage( method: HttpMethod.Get, @@ -74,7 +73,7 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplication string mediaType, string errorMessage) { - HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + HttpResponseMessage responseMessage = new(statusCode: HttpStatusCode.NotFound) { RequestMessage = new HttpRequestMessage( method: HttpMethod.Get, @@ -92,11 +91,11 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplication Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); - Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway.")); + Assert.IsNotNull(value: documentClientException.Message); Assert.IsNotNull(value: documentClientException.Error); Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); - Assert.IsTrue(documentClientException.Error.Message.Contains("No response content from gateway.")); + Assert.IsNotNull(value: documentClientException.Error.Message); } /// @@ -112,7 +111,7 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplication string mediaType, string contentMessage) { - HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + HttpResponseMessage responseMessage = new(statusCode: HttpStatusCode.NotFound) { RequestMessage = new HttpRequestMessage( method: HttpMethod.Get, @@ -124,16 +123,16 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplication }; DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( - responseMessage: responseMessage, - requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); + responseMessage: responseMessage, + requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); - Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway.")); + Assert.IsNotNull(value: documentClientException.Message); Assert.IsNotNull(value: documentClientException.Error); Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); - Assert.IsTrue(documentClientException.Error.Message.Contains("No response content from gateway.")); + Assert.IsNotNull(value: documentClientException.Error.Message); } /// @@ -148,7 +147,7 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso string mediaType, string errorMessage) { - HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + HttpResponseMessage responseMessage = new(statusCode: HttpStatusCode.NotFound) { RequestMessage = new HttpRequestMessage( method: HttpMethod.Get, @@ -166,11 +165,11 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); - Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway.")); + Assert.IsNotNull(value: documentClientException.Message); Assert.IsNotNull(value: documentClientException.Error); Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); - Assert.AreEqual(expected: "No response content from gateway.", actual: documentClientException.Error.Message); + Assert.IsNotNull(value: documentClientException.Error.Message); } /// @@ -192,7 +191,7 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso string mediaType, string contentMessage) { - HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + HttpResponseMessage responseMessage = new(statusCode: HttpStatusCode.NotFound) { RequestMessage = new HttpRequestMessage( method: HttpMethod.Get, @@ -204,8 +203,8 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso }; DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( - responseMessage: responseMessage, - requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); + responseMessage: responseMessage, + requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); @@ -223,7 +222,7 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso string mediaType, string contentMessage) { - HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + HttpResponseMessage responseMessage = new(statusCode: HttpStatusCode.NotFound) { RequestMessage = new HttpRequestMessage( method: HttpMethod.Get, @@ -235,16 +234,16 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso }; DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync( - responseMessage: responseMessage, - requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); + responseMessage: responseMessage, + requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()); Assert.IsNotNull(value: documentClientException); Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode); - Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway.")); + Assert.IsNotNull(value: documentClientException.Message); Assert.IsNotNull(value: documentClientException.Error); Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code); - Assert.IsTrue(documentClientException.Error.Message.Contains("No response content from gateway.")); + Assert.IsNotNull(value: documentClientException.Error.Message); } /// @@ -255,8 +254,8 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso public async Task TestCreateDocumentClientExceptionWhenResponseMessageIsNullExpectsArgumentNullException() { ArgumentNullException argumentNullException = await Assert.ThrowsExceptionAsync(async () => await GatewayStoreClient.CreateDocumentClientExceptionAsync( - responseMessage: default, - requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()) + responseMessage: default, + requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()) ); Assert.IsNotNull(argumentNullException); @@ -270,7 +269,7 @@ public async Task TestCreateDocumentClientExceptionWhenResponseMessageIsNullExpe [Owner("philipthomas-MSFT")] public async Task TestCreateDocumentClientExceptionWhenRequestStatisticsIsNullExpectsArgumentNullException() { - HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound) + HttpResponseMessage responseMessage = new(statusCode: HttpStatusCode.NotFound) { RequestMessage = new HttpRequestMessage( method: HttpMethod.Get, @@ -281,8 +280,8 @@ public async Task TestCreateDocumentClientExceptionWhenRequestStatisticsIsNullEx }; ArgumentNullException argumentNullException = await Assert.ThrowsExceptionAsync(async () => await GatewayStoreClient.CreateDocumentClientExceptionAsync( - responseMessage: responseMessage, - requestStatistics: default) + responseMessage: responseMessage, + requestStatistics: default) ); Assert.IsNotNull(argumentNullException); From 0e158a8e8f3e6d1be4b37fdf297b4f8c44c592c7 Mon Sep 17 00:00:00 2001 From: philipthomas Date: Tue, 9 Jan 2024 09:52:12 -0500 Subject: [PATCH 14/15] removed null check on 2 arguments because it was breaking other tests. I will maybe come back to it later. --- .../src/GatewayStoreClient.cs | 10 ----- .../GatewayStoreClientTests.cs | 42 ------------------- 2 files changed, 52 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index 35ca1dcd28..d90dac1e7c 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -140,16 +140,6 @@ internal static async Task CreateDocumentClientExceptio HttpResponseMessage responseMessage, IClientSideRequestStatistics requestStatistics) { - if (responseMessage is null) - { - throw new ArgumentNullException(nameof(responseMessage)); - } - - if (requestStatistics is null) - { - throw new ArgumentNullException(nameof(requestStatistics)); - } - if (!PathsHelper.TryParsePathSegments( resourceUrl: responseMessage.RequestMessage.RequestUri.LocalPath, isFeed: out _, diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs index 3a537233b7..ff750f8ed6 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs @@ -246,48 +246,6 @@ public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJso Assert.IsNotNull(value: documentClientException.Error.Message); } - /// - /// Testing CreateDocumentClientExceptionAsync when response message argument is null, then expects an argumentNullException. - /// - [TestMethod] - [Owner("philipthomas-MSFT")] - public async Task TestCreateDocumentClientExceptionWhenResponseMessageIsNullExpectsArgumentNullException() - { - ArgumentNullException argumentNullException = await Assert.ThrowsExceptionAsync(async () => await GatewayStoreClient.CreateDocumentClientExceptionAsync( - responseMessage: default, - requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics()) - ); - - Assert.IsNotNull(argumentNullException); - Assert.AreEqual(expected: "Value cannot be null. (Parameter 'responseMessage')", actual: argumentNullException.Message); - } - - /// - /// Testing CreateDocumentClientExceptionAsync when request statistics argument is null, then expects an argumentNullException. - /// - [TestMethod] - [Owner("philipthomas-MSFT")] - public async Task TestCreateDocumentClientExceptionWhenRequestStatisticsIsNullExpectsArgumentNullException() - { - HttpResponseMessage responseMessage = new(statusCode: HttpStatusCode.NotFound) - { - RequestMessage = new HttpRequestMessage( - method: HttpMethod.Get, - requestUri: @"https://pt_ac_test_uri.com/"), - Content = new StringContent( - content: JsonConvert.SerializeObject( - value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = "" })), - }; - - ArgumentNullException argumentNullException = await Assert.ThrowsExceptionAsync(async () => await GatewayStoreClient.CreateDocumentClientExceptionAsync( - responseMessage: responseMessage, - requestStatistics: default) - ); - - Assert.IsNotNull(argumentNullException); - Assert.AreEqual(expected: "Value cannot be null. (Parameter 'requestStatistics')", actual: argumentNullException.Message); - } - private static IClientSideRequestStatistics CreateClientSideRequestStatistics() { return new ClientSideRequestStatisticsTraceDatum( From 3d258621a1f2dc1533b135b35b5251fcc5e9f96e Mon Sep 17 00:00:00 2001 From: philipthomas Date: Tue, 9 Jan 2024 10:50:25 -0500 Subject: [PATCH 15/15] && and > 0 in the conditional --- Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs index d90dac1e7c..db6f4a027f 100644 --- a/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs +++ b/Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs @@ -151,8 +151,8 @@ internal static async Task CreateDocumentClientExceptio } // If service rejects the initial payload like header is to large it will return an HTML error instead of JSON. - if (string.Equals(responseMessage.Content?.Headers?.ContentType?.MediaType, "application/json", StringComparison.OrdinalIgnoreCase) || - responseMessage.Content?.Headers.ContentLength != 0) + if (string.Equals(responseMessage.Content?.Headers?.ContentType?.MediaType, "application/json", StringComparison.OrdinalIgnoreCase) && + responseMessage.Content?.Headers.ContentLength > 0) { try {