From fd9ad862e0d36da4173d2a9dc078b9e06ed8863b Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Wed, 1 Dec 2021 11:24:11 -0800 Subject: [PATCH] Add tests for RequestFailedException constructor (#25578) * Tests + Bug Fix * initialize sanitizer on response; make IsError virtual * add test * add more tests * update TestFramework to use a project reference * consider parent result * pr fb * This-works * Add Core project reference to EventHubs solution * update API Co-authored-by: Pavel Krymets --- eng/Directory.Build.Common.props | 5 -- eng/Directory.Build.Common.targets | 3 + .../src/Azure.Core.TestFramework.csproj | 2 +- .../src/MockResponse.cs | 4 + sdk/core/Azure.Core/api/Azure.Core.net461.cs | 2 +- sdk/core/Azure.Core/api/Azure.Core.net5.0.cs | 2 +- .../api/Azure.Core.netcoreapp2.1.cs | 2 +- .../api/Azure.Core.netstandard2.0.cs | 2 +- .../src/Pipeline/HttpPipelineBuilder.cs | 2 +- sdk/core/Azure.Core/src/Response.cs | 4 +- .../src/Shared/HttpMessageSanitizer.cs | 2 + .../tests/FailedResponseExceptionTests.cs | 88 ++++++++++++++++++- sdk/core/Azure.Core/tests/ResponseTests.cs | 25 ++++++ .../Azure.Messaging.EventHubs.sln | 17 +++- 14 files changed, 145 insertions(+), 15 deletions(-) mode change 100755 => 100644 sdk/eventhub/Azure.Messaging.EventHubs/Azure.Messaging.EventHubs.sln diff --git a/eng/Directory.Build.Common.props b/eng/Directory.Build.Common.props index 0abadd8c514a..d4c9519e9da9 100644 --- a/eng/Directory.Build.Common.props +++ b/eng/Directory.Build.Common.props @@ -135,11 +135,6 @@ - - - - - $(RequiredTargetFrameworks) diff --git a/eng/Directory.Build.Common.targets b/eng/Directory.Build.Common.targets index 058e13d831eb..9f7cb6f5ea7f 100644 --- a/eng/Directory.Build.Common.targets +++ b/eng/Directory.Build.Common.targets @@ -181,8 +181,11 @@ + + + diff --git a/sdk/core/Azure.Core.TestFramework/src/Azure.Core.TestFramework.csproj b/sdk/core/Azure.Core.TestFramework/src/Azure.Core.TestFramework.csproj index c213cbb84330..35d0ce18be88 100644 --- a/sdk/core/Azure.Core.TestFramework/src/Azure.Core.TestFramework.csproj +++ b/sdk/core/Azure.Core.TestFramework/src/Azure.Core.TestFramework.csproj @@ -4,7 +4,7 @@ - + diff --git a/sdk/core/Azure.Core.TestFramework/src/MockResponse.cs b/sdk/core/Azure.Core.TestFramework/src/MockResponse.cs index 77e1e211fde5..fb950ce1408b 100644 --- a/sdk/core/Azure.Core.TestFramework/src/MockResponse.cs +++ b/sdk/core/Azure.Core.TestFramework/src/MockResponse.cs @@ -27,6 +27,10 @@ public MockResponse(int status, string reasonPhrase = null) public override string ClientRequestId { get; set; } + private bool? _isError; + public override bool IsError { get => _isError ?? base.IsError; } + public void SetIsError(bool value) => _isError = value; + public bool IsDisposed { get; private set; } public void SetContent(byte[] content) diff --git a/sdk/core/Azure.Core/api/Azure.Core.net461.cs b/sdk/core/Azure.Core/api/Azure.Core.net461.cs index 8d7058718d8b..2d2e35b1bb3e 100644 --- a/sdk/core/Azure.Core/api/Azure.Core.net461.cs +++ b/sdk/core/Azure.Core/api/Azure.Core.net461.cs @@ -217,7 +217,7 @@ protected Response() { } public virtual System.BinaryData Content { get { throw null; } } public abstract System.IO.Stream? ContentStream { get; set; } public virtual Azure.Core.ResponseHeaders Headers { get { throw null; } } - public bool IsError { get { throw null; } } + public virtual bool IsError { get { throw null; } } public abstract string ReasonPhrase { get; } public abstract int Status { get; } protected internal abstract bool ContainsHeader(string name); diff --git a/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs b/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs index c25d9a2641b2..37dd744c51af 100644 --- a/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs +++ b/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs @@ -217,7 +217,7 @@ protected Response() { } public virtual System.BinaryData Content { get { throw null; } } public abstract System.IO.Stream? ContentStream { get; set; } public virtual Azure.Core.ResponseHeaders Headers { get { throw null; } } - public bool IsError { get { throw null; } } + public virtual bool IsError { get { throw null; } } public abstract string ReasonPhrase { get; } public abstract int Status { get; } protected internal abstract bool ContainsHeader(string name); diff --git a/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs b/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs index 8d7058718d8b..2d2e35b1bb3e 100644 --- a/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs +++ b/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs @@ -217,7 +217,7 @@ protected Response() { } public virtual System.BinaryData Content { get { throw null; } } public abstract System.IO.Stream? ContentStream { get; set; } public virtual Azure.Core.ResponseHeaders Headers { get { throw null; } } - public bool IsError { get { throw null; } } + public virtual bool IsError { get { throw null; } } public abstract string ReasonPhrase { get; } public abstract int Status { get; } protected internal abstract bool ContainsHeader(string name); diff --git a/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs b/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs index 8d7058718d8b..2d2e35b1bb3e 100644 --- a/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs +++ b/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs @@ -217,7 +217,7 @@ protected Response() { } public virtual System.BinaryData Content { get { throw null; } } public abstract System.IO.Stream? ContentStream { get; set; } public virtual Azure.Core.ResponseHeaders Headers { get { throw null; } } - public bool IsError { get { throw null; } } + public virtual bool IsError { get { throw null; } } public abstract string ReasonPhrase { get; } public abstract int Status { get; } protected internal abstract bool ContainsHeader(string name); diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index 165d51652063..d7f4bb2d97d9 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -84,7 +84,7 @@ void AddCustomerPolicies(HttpPipelinePosition position) DiagnosticsOptions diagnostics = options.Diagnostics; - var sanitizer = new HttpMessageSanitizer(diagnostics.LoggedHeaderNames.ToArray(), diagnostics.LoggedQueryParameters.ToArray()); + var sanitizer = new HttpMessageSanitizer(diagnostics.LoggedQueryParameters.ToArray(), diagnostics.LoggedHeaderNames.ToArray()); bool isDistributedTracingEnabled = options.Diagnostics.IsDistributedTracingEnabled; diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index 64eb6f1afd4a..57aa065640b7 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -86,9 +86,9 @@ public virtual BinaryData Content /// Indicates whether the message's considers this /// response an error. /// - public bool IsError { get; internal set; } + public virtual bool IsError { get; internal set; } - internal HttpMessageSanitizer? Sanitizer { get; set; } + internal HttpMessageSanitizer? Sanitizer { get; set; } = HttpMessageSanitizer.Default; /// /// Returns header value if the header is stored in the collection. If header has multiple values they are going to be joined with a comma. diff --git a/sdk/core/Azure.Core/src/Shared/HttpMessageSanitizer.cs b/sdk/core/Azure.Core/src/Shared/HttpMessageSanitizer.cs index 7a1d4d66ce43..2908c9ffacd6 100644 --- a/sdk/core/Azure.Core/src/Shared/HttpMessageSanitizer.cs +++ b/sdk/core/Azure.Core/src/Shared/HttpMessageSanitizer.cs @@ -17,6 +17,8 @@ internal class HttpMessageSanitizer private readonly string _redactedPlaceholder; private readonly HashSet _allowedHeaders; + internal static HttpMessageSanitizer Default = new HttpMessageSanitizer(Array.Empty(), Array.Empty()); + public HttpMessageSanitizer(string[] allowedQueryParameters, string[] allowedHeaders, string redactedPlaceholder = "REDACTED") { _logAllHeaders = allowedHeaders.Contains(LogAllValue); diff --git a/sdk/core/Azure.Core/tests/FailedResponseExceptionTests.cs b/sdk/core/Azure.Core/tests/FailedResponseExceptionTests.cs index 98bf2e3dad36..4cc913481dfc 100644 --- a/sdk/core/Azure.Core/tests/FailedResponseExceptionTests.cs +++ b/sdk/core/Azure.Core/tests/FailedResponseExceptionTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Runtime.Serialization; using System.Threading.Tasks; using Azure.Core.Pipeline; @@ -16,6 +17,7 @@ public class FailedResponseExceptionTests { private static readonly string s_nl = Environment.NewLine; private static ClientDiagnostics ClientDiagnostics = new ClientDiagnostics(new TestClientOption()); + private static HttpMessageSanitizer Sanitizer = new TestClientOption().Sanitizer; [Test] public async Task FormatsResponse() @@ -36,6 +38,45 @@ public async Task FormatsResponse() Assert.AreEqual(formattedResponse, exception.Message); } + [Test] + public void FormatsResponse_ResponseCtor() + { + var formattedResponse = + "Service request failed." + s_nl + + "Status: 210 (Reason)" + s_nl + + s_nl + + "Headers:" + s_nl + + "Custom-Header: Value" + s_nl + + "x-ms-requestId: 123" + s_nl; + + var response = new MockResponse(210, "Reason"); + response.AddHeader(new HttpHeader("Custom-Header", "Value")); + response.AddHeader(new HttpHeader("x-ms-requestId", "123")); + response.Sanitizer = Sanitizer; + + RequestFailedException exception = new RequestFailedException(response); + Assert.AreEqual(formattedResponse, exception.Message); + } + + [Test] + public void FormatsResponseWithoutSanitizer_ResponseCtor() + { + var formattedResponse = + "Service request failed." + s_nl + + "Status: 210 (Reason)" + s_nl + + s_nl + + "Headers:" + s_nl + + "Custom-Header: REDACTED" + s_nl + + "x-ms-requestId: REDACTED" + s_nl; + + var response = new MockResponse(210, "Reason"); + response.AddHeader(new HttpHeader("Custom-Header", "Value")); + response.AddHeader(new HttpHeader("x-ms-requestId", "123")); + + RequestFailedException exception = new RequestFailedException(response); + Assert.AreEqual(formattedResponse, exception.Message); + } + [Test] public async Task HeadersAreSanitized() { @@ -55,6 +96,25 @@ public async Task HeadersAreSanitized() Assert.AreEqual(formattedResponse, exception.Message); } + [Test] + public void HeadersAreSanitized_ResponseCtor() + { + var formattedResponse = + "Service request failed." + s_nl + + "Status: 210 (Reason)" + s_nl + + s_nl + + "Headers:" + s_nl + + "Custom-Header-2: REDACTED" + s_nl + + "x-ms-requestId-2: REDACTED" + s_nl; + + var response = new MockResponse(210, "Reason"); + response.AddHeader(new HttpHeader("Custom-Header-2", "Value")); + response.AddHeader(new HttpHeader("x-ms-requestId-2", "123")); + + RequestFailedException exception = new RequestFailedException(response); + Assert.AreEqual(formattedResponse, exception.Message); + } + [Test] public async Task FormatsResponseContentForTextContentTypes() { @@ -176,7 +236,7 @@ public void RequestFailedExceptionIsSerializeable() using var memoryStream = new MemoryStream(); dataContractSerializer.WriteObject(memoryStream, exception); memoryStream.Position = 0; - deserialized = (RequestFailedException) dataContractSerializer.ReadObject(memoryStream); + deserialized = (RequestFailedException)dataContractSerializer.ReadObject(memoryStream); Assert.AreEqual(exception.Message, deserialized.Message); Assert.AreEqual(exception.Status, deserialized.Status); @@ -206,6 +266,30 @@ public async Task ParsesJsonErrors() Assert.AreEqual("StatusCode", exception.ErrorCode); } + [Test] + public void ParsesJsonErrors_ResponseCtor() + { + var formattedResponse = + "Custom message" + s_nl + + "Status: 210 (Reason)" + s_nl + + "ErrorCode: StatusCode" + s_nl + + s_nl + + "Content:" + s_nl + + "{ \"error\": { \"code\":\"StatusCode\", \"message\":\"Custom message\" }}" + s_nl + + s_nl + + "Headers:" + s_nl + + "Content-Type: text/json" + s_nl; + + var response = new MockResponse(210, "Reason"); + response.SetContent("{ \"error\": { \"code\":\"StatusCode\", \"message\":\"Custom message\" }}"); + response.AddHeader(new HttpHeader("Content-Type", "text/json")); + response.Sanitizer = Sanitizer; + + RequestFailedException exception = new RequestFailedException(response); + Assert.AreEqual(formattedResponse, exception.Message); + Assert.AreEqual("StatusCode", exception.ErrorCode); + } + [Test] public async Task IgnoresInvalidJsonErrors() { @@ -250,6 +334,8 @@ public async Task IgnoresNonStandardJson() private class TestClientOption : ClientOptions { + public HttpMessageSanitizer Sanitizer => new HttpMessageSanitizer(Diagnostics.LoggedQueryParameters.ToArray(), Diagnostics.LoggedHeaderNames.ToArray()); + public TestClientOption() { Diagnostics.LoggedHeaderNames.Add("x-ms-requestId"); diff --git a/sdk/core/Azure.Core/tests/ResponseTests.cs b/sdk/core/Azure.Core/tests/ResponseTests.cs index a83ab5e63393..32d5bbd3c43f 100644 --- a/sdk/core/Azure.Core/tests/ResponseTests.cs +++ b/sdk/core/Azure.Core/tests/ResponseTests.cs @@ -6,6 +6,7 @@ using System.Linq; using Azure.Core.TestFramework; using NUnit.Framework; +using Moq; namespace Azure.Core.Tests { @@ -145,6 +146,30 @@ public void ContentPropertyWorksForMemoryStreamsWithPrivateBuffers() CollectionAssert.AreEqual(responseBody, response.Content.ToArray()); } + [Test] + public void CanMockIsError() + { + var response = new MockResponse(500); + + Assert.IsFalse(response.IsError); + + response.SetIsError(true); + + Assert.IsTrue(response.IsError); + } + + [Test] + public void CanMoqIsError() + { + var response = new Mock(); + + Assert.IsFalse(response.Object.IsError); + + response.SetupGet(x => x.IsError).Returns(true); + + Assert.IsTrue(response.Object.IsError); + } + internal class TestPayload { public string Name { get; } diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/Azure.Messaging.EventHubs.sln b/sdk/eventhub/Azure.Messaging.EventHubs/Azure.Messaging.EventHubs.sln old mode 100755 new mode 100644 index 6936bc419f3b..9d1600099858 --- a/sdk/eventhub/Azure.Messaging.EventHubs/Azure.Messaging.EventHubs.sln +++ b/sdk/eventhub/Azure.Messaging.EventHubs/Azure.Messaging.EventHubs.sln @@ -11,6 +11,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core.TestFramework", EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "External", "External", "{2DB233D3-E757-423C-8F8D-742B0AFF4713}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core", "..\..\core\Azure.Core\src\Azure.Core.csproj", "{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}" +EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Azure.Core.Experimental", "..\..\core\Azure.Core.Experimental\src\Azure.Core.Experimental.csproj", "{FB10A6B3-88DD-4296-9241-53ECDF942842}" EndProject Global @@ -59,6 +61,18 @@ Global {2CFDB3D6-5CFB-428C-9C89-29DD169B5433}.Release|x64.Build.0 = Release|Any CPU {2CFDB3D6-5CFB-428C-9C89-29DD169B5433}.Release|x86.ActiveCfg = Release|Any CPU {2CFDB3D6-5CFB-428C-9C89-29DD169B5433}.Release|x86.Build.0 = Release|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|Any CPU.Build.0 = Debug|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|x64.ActiveCfg = Debug|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|x64.Build.0 = Debug|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|x86.ActiveCfg = Debug|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|x86.Build.0 = Debug|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|Any CPU.ActiveCfg = Release|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|Any CPU.Build.0 = Release|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|x64.ActiveCfg = Release|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|x64.Build.0 = Release|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|x86.ActiveCfg = Release|Any CPU + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|x86.Build.0 = Release|Any CPU {FB10A6B3-88DD-4296-9241-53ECDF942842}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {FB10A6B3-88DD-4296-9241-53ECDF942842}.Debug|Any CPU.Build.0 = Debug|Any CPU {FB10A6B3-88DD-4296-9241-53ECDF942842}.Debug|x64.ActiveCfg = Debug|Any CPU @@ -70,13 +84,14 @@ Global {FB10A6B3-88DD-4296-9241-53ECDF942842}.Release|x64.ActiveCfg = Release|Any CPU {FB10A6B3-88DD-4296-9241-53ECDF942842}.Release|x64.Build.0 = Release|Any CPU {FB10A6B3-88DD-4296-9241-53ECDF942842}.Release|x86.ActiveCfg = Release|Any CPU - {FB10A6B3-88DD-4296-9241-53ECDF942842}.Release|x86.Build.0 = Release|Any CPU + {FB10A6B3-88DD-4296-9241-53ECDF942842}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE EndGlobalSection GlobalSection(NestedProjects) = preSolution {2CFDB3D6-5CFB-428C-9C89-29DD169B5433} = {2DB233D3-E757-423C-8F8D-742B0AFF4713} + {6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3} = {2DB233D3-E757-423C-8F8D-742B0AFF4713} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {509F2EE0-3348-4506-8AC7-9945B602CB43}