From cb7e8aff68994e97a580a9963e0878ee4baeb01a Mon Sep 17 00:00:00 2001 From: maliming Date: Tue, 16 Jun 2026 18:24:12 +0800 Subject: [PATCH] Refine ClientProxyBase Accept handling and narrow remote stream cast --- .../Client/ClientProxying/ClientProxyBase.cs | 60 +++++----- .../ReturnValueApiDescriptionModel.cs | 23 +--- .../ClientProxyBase_ContentTypes_Tests.cs | 103 ++++++++++++++++-- .../DynamicProxying/IRegularTestController.cs | 2 + .../DynamicProxying/RegularTestController.cs | 7 ++ ...lerClientProxy_ReturnContentTypes_Tests.cs | 15 +-- .../ReturnValueApiDescriptionModel_Tests.cs | 34 +++--- 7 files changed, 153 insertions(+), 91 deletions(-) diff --git a/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/ClientProxying/ClientProxyBase.cs b/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/ClientProxying/ClientProxyBase.cs index e3d4a06dfb..0b7a21c71e 100644 --- a/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/ClientProxying/ClientProxyBase.cs +++ b/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/ClientProxying/ClientProxyBase.cs @@ -108,7 +108,7 @@ public class ClientProxyBase : ITransientDependency { var responseContent = await RequestAsync(requestContext); - if (typeof(T).IsAssignableFrom(typeof(RemoteStreamContent))) + if (typeof(T) == typeof(IRemoteStreamContent) || typeof(T) == typeof(RemoteStreamContent)) { /* returning a class that holds a reference to response * content just to be sure that GC does not dispose of @@ -153,7 +153,6 @@ public class ClientProxyBase : ITransientDependency try { - // JSON null literal deserializes to null — preserve that as empty string for callers expecting non-null. var parsed = JsonSerializer.Deserialize(body); return parsed ?? string.Empty; } @@ -170,7 +169,7 @@ public class ClientProxyBase : ITransientDependency return string.Empty; } var semi = mediaType.IndexOf(';'); - return (semi < 0 ? mediaType : mediaType.Substring(0, semi)).Trim(); + return (semi < 0 ? mediaType : mediaType.Substring(0, semi)).Trim().ToLowerInvariant(); } protected virtual async Task RequestAsync(ClientProxyRequestContext requestContext) @@ -360,24 +359,7 @@ public class ClientProxyBase : ITransientDependency HttpRequestMessage requestMessage, ApiVersionInfo apiVersion) { - //API Version - if (!apiVersion.Version.IsNullOrEmpty()) - { - //TODO: What about other media types? - requestMessage.Headers.Add("accept", $"{MimeTypes.Text.Plain}; v={apiVersion.Version}"); - requestMessage.Headers.Add("accept", $"{MimeTypes.Application.Json}; v={apiVersion.Version}"); - requestMessage.Headers.Add("api-version", apiVersion.Version); - } - - //Return-type-aware Accept header (only when none already set) - if (!requestMessage.Headers.Contains("accept")) - { - var acceptForReturn = GetAcceptForActionReturn(action); - if (!acceptForReturn.IsNullOrEmpty()) - { - requestMessage.Headers.Add("accept", acceptForReturn); - } - } + AddAcceptHeaders(action, requestMessage, apiVersion); //Header parameters var headers = action.Parameters.Where(p => p.BindingSourceId == ParameterBindingSources.Header).ToArray(); @@ -422,6 +404,30 @@ public class ClientProxyBase : ITransientDependency } } + protected virtual void AddAcceptHeaders( + ActionApiDescriptionModel action, + HttpRequestMessage requestMessage, + ApiVersionInfo apiVersion) + { + var acceptForReturn = GetAcceptForActionReturn(action); + var versionSuffix = apiVersion.Version.IsNullOrEmpty() ? string.Empty : $"; v={apiVersion.Version}"; + + if (!acceptForReturn.IsNullOrEmpty()) + { + requestMessage.Headers.Add("accept", acceptForReturn + versionSuffix); + } + else + { + requestMessage.Headers.Add("accept", MimeTypes.Text.Plain + versionSuffix); + requestMessage.Headers.Add("accept", MimeTypes.Application.Json + versionSuffix); + } + + if (!apiVersion.Version.IsNullOrEmpty()) + { + requestMessage.Headers.Add("api-version", apiVersion.Version); + } + } + protected virtual string? GetAcceptForActionReturn(ActionApiDescriptionModel action) { if (action.ReturnValue.IsRemoteStream || @@ -439,17 +445,7 @@ public class ClientProxyBase : ITransientDependency var normalized = contentTypes.Select(NormalizeMediaType).ToList(); - if (normalized.Any(IsJsonMediaType)) - { - return MimeTypes.Application.Json; - } - - if (normalized.All(ct => ct.StartsWith("text/", StringComparison.OrdinalIgnoreCase))) - { - return MimeTypes.Text.Plain; - } - - return null; + return normalized.FirstOrDefault(IsJsonMediaType) ?? normalized[0]; } private static bool IsJsonMediaType(string normalizedMediaType) diff --git a/framework/src/Volo.Abp.Http/Volo/Abp/Http/Modeling/ReturnValueApiDescriptionModel.cs b/framework/src/Volo.Abp.Http/Volo/Abp/Http/Modeling/ReturnValueApiDescriptionModel.cs index dcd1b16e6b..25b2fb149a 100644 --- a/framework/src/Volo.Abp.Http/Volo/Abp/Http/Modeling/ReturnValueApiDescriptionModel.cs +++ b/framework/src/Volo.Abp.Http/Volo/Abp/Http/Modeling/ReturnValueApiDescriptionModel.cs @@ -1,5 +1,4 @@ using System; -using System.Collections; using System.Collections.Generic; using Volo.Abp.Content; using Volo.Abp.Reflection; @@ -40,26 +39,6 @@ public class ReturnValueApiDescriptionModel private static bool IsRemoteStreamType(Type type) { - if (typeof(IRemoteStreamContent).IsAssignableFrom(type)) - { - return true; - } - - if (type.IsArray && type.GetElementType() is { } elementType && - typeof(IRemoteStreamContent).IsAssignableFrom(elementType)) - { - return true; - } - - if (typeof(IEnumerable).IsAssignableFrom(type) && type.IsGenericType) - { - var genericArg = type.GetGenericArguments()[0]; - if (typeof(IRemoteStreamContent).IsAssignableFrom(genericArg)) - { - return true; - } - } - - return false; + return type == typeof(IRemoteStreamContent) || type == typeof(RemoteStreamContent); } } diff --git a/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/Client/ClientProxying/ClientProxyBase_ContentTypes_Tests.cs b/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/Client/ClientProxying/ClientProxyBase_ContentTypes_Tests.cs index b2acdfedbb..c30fbb0d43 100644 --- a/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/Client/ClientProxying/ClientProxyBase_ContentTypes_Tests.cs +++ b/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/Client/ClientProxying/ClientProxyBase_ContentTypes_Tests.cs @@ -1,5 +1,7 @@ #nullable enable using System.Collections.Generic; +using System.Linq; +using System.Net.Http; using Shouldly; using Volo.Abp.Content; using Volo.Abp.Http.Modeling; @@ -57,13 +59,13 @@ public class ClientProxyBase_GetAcceptForActionReturn_Tests } [Fact] - public void Mixed_Text_And_Octet_Stream_Should_Not_Pick_TextPlain() + public void Mixed_Text_And_Octet_Stream_Should_Echo_First_Content_Type() { var action = BuildAction( returnType: "System.String", contentTypes: new[] { "text/plain", "application/octet-stream" }); - InvokeGetAcceptForActionReturn(action).ShouldBeNull(); + InvokeGetAcceptForActionReturn(action).ShouldBe("text/plain"); } [Fact] @@ -77,23 +79,33 @@ public class ClientProxyBase_GetAcceptForActionReturn_Tests } [Fact] - public void Single_TextHtml_Should_Pick_TextPlain() + public void Single_TextHtml_Should_Echo_Back_TextHtml() { var action = BuildAction( returnType: "System.String", contentTypes: new[] { "text/html" }); - InvokeGetAcceptForActionReturn(action).ShouldBe("text/plain"); + InvokeGetAcceptForActionReturn(action).ShouldBe("text/html"); } [Fact] - public void OctetStream_Only_With_ObjectReturn_Should_Return_Null() + public void OctetStream_Only_With_ObjectReturn_Should_Echo_OctetStream() { var action = BuildAction( returnType: "My.Project.UserDto", contentTypes: new[] { "application/octet-stream" }); - InvokeGetAcceptForActionReturn(action).ShouldBeNull(); + InvokeGetAcceptForActionReturn(action).ShouldBe("application/octet-stream"); + } + + [Fact] + public void ApplicationXml_Only_Should_Echo_Back_Xml_Instead_Of_Legacy_Pair() + { + var action = BuildAction( + returnType: "My.Project.SoapEnvelope", + contentTypes: new[] { "application/xml" }); + + InvokeGetAcceptForActionReturn(action).ShouldBe("application/xml"); } [Fact] @@ -127,23 +139,33 @@ public class ClientProxyBase_GetAcceptForActionReturn_Tests } [Fact] - public void Text_Json_Should_Be_Treated_As_Json() + public void Text_Json_Should_Echo_Back_Text_Json() { var action = BuildAction( returnType: "System.String", contentTypes: new[] { "text/json" }); - InvokeGetAcceptForActionReturn(action).ShouldBe("application/json"); + InvokeGetAcceptForActionReturn(action).ShouldBe("text/json"); } [Fact] - public void Application_Problem_Json_Should_Be_Treated_As_Json() + public void Application_Problem_Json_Should_Echo_Back_The_Plus_Json_Variant() { var action = BuildAction( returnType: "System.String", contentTypes: new[] { "application/problem+json" }); - InvokeGetAcceptForActionReturn(action).ShouldBe("application/json"); + InvokeGetAcceptForActionReturn(action).ShouldBe("application/problem+json"); + } + + [Fact] + public void Vendor_Plus_Json_Should_Echo_Back_The_Plus_Json_Variant() + { + var action = BuildAction( + returnType: "System.String", + contentTypes: new[] { "application/vnd.api+json" }); + + InvokeGetAcceptForActionReturn(action).ShouldBe("application/vnd.api+json"); } [Fact] @@ -184,9 +206,70 @@ public class ClientProxyBase_GetAcceptForActionReturn_Tests }; } + [Fact] + public void AddHeaders_With_ApiVersion_Should_Combine_OctetStream_Accept_With_Version_Suffix() + { + var action = BuildAction( + returnType: typeof(IRemoteStreamContent).FullName!, + contentTypes: new[] { "application/json", "text/plain" }); + + var headers = InvokeAddHeadersAndCollectAccept(action, version: "2.0"); + + headers.ShouldContain("application/octet-stream; v=2.0"); + headers.ShouldNotContain(h => h == "text/plain; v=2.0"); + headers.ShouldNotContain(h => h == "application/json; v=2.0"); + } + + [Fact] + public void AddHeaders_Without_ApiVersion_Should_Emit_OctetStream_For_Stream_Returns() + { + var action = BuildAction( + returnType: typeof(IRemoteStreamContent).FullName!, + contentTypes: new[] { "application/json" }); + + var headers = InvokeAddHeadersAndCollectAccept(action, version: null); + + headers.ShouldContain("application/octet-stream"); + } + + [Fact] + public void AddHeaders_Without_ContentType_Metadata_Should_Fall_Back_To_Text_And_Json_Pair() + { + var action = BuildAction(returnType: "System.Int32", contentTypes: null); + + var headers = InvokeAddHeadersAndCollectAccept(action, version: "1.0"); + + headers.ShouldContain("text/plain; v=1.0"); + headers.ShouldContain("application/json; v=1.0"); + } + + [Fact] + public void AddHeaders_Without_ApiVersion_And_Without_ContentType_Metadata_Should_Emit_Unversioned_Text_Json_Pair() + { + var action = BuildAction(returnType: "System.Int32", contentTypes: null); + + var headers = InvokeAddHeadersAndCollectAccept(action, version: null); + + headers.ShouldContain("text/plain"); + headers.ShouldContain("application/json"); + headers.ShouldNotContain(h => h.Contains("; v=")); + } + + private static IList InvokeAddHeadersAndCollectAccept(ActionApiDescriptionModel action, string? version) + { + var proxy = new TestableClientProxy(); + var message = new HttpRequestMessage(HttpMethod.Get, "http://localhost/x"); + var apiVersion = new ApiVersionInfo("HeaderModelBinding", version ?? string.Empty); + proxy.PublicAddAcceptHeaders(action, message, apiVersion); + return message.Headers.Accept.Select(a => a.ToString()).ToList(); + } + private sealed class TestableClientProxy : ClientProxyBase { public string? PublicGetAcceptForActionReturn(ActionApiDescriptionModel action) => GetAcceptForActionReturn(action); + + public void PublicAddAcceptHeaders(ActionApiDescriptionModel action, HttpRequestMessage requestMessage, ApiVersionInfo apiVersion) + => AddAcceptHeaders(action, requestMessage, apiVersion); } } diff --git a/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/IRegularTestController.cs b/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/IRegularTestController.cs index 5be89375a0..fdf279e1ce 100644 --- a/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/IRegularTestController.cs +++ b/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/IRegularTestController.cs @@ -25,6 +25,8 @@ public interface IRegularTestController Task DownloadIconAsync(); + Task GetReferenceTypeObjectAsync(); + Task GetByteArrayAsync(); Task GetException1Async(); diff --git a/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/RegularTestController.cs b/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/RegularTestController.cs index 437d047b71..1d0864991b 100644 --- a/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/RegularTestController.cs +++ b/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/RegularTestController.cs @@ -85,6 +85,13 @@ public class RegularTestController : AbpController, IRegularTestController new RemoteStreamContent(new MemoryStream(bytes), "icon.bin", "application/octet-stream")); } + [HttpGet] + [Route("reference-type-object")] + public Task GetReferenceTypeObjectAsync() + { + return Task.FromResult(new Car { Year = 1999, Model = "BMW" }); + } + [HttpGet] [Route("byte-array")] public Task GetByteArrayAsync() diff --git a/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/RegularTestControllerClientProxy_ReturnContentTypes_Tests.cs b/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/RegularTestControllerClientProxy_ReturnContentTypes_Tests.cs index a6725d6e6c..92c8b5b23c 100644 --- a/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/RegularTestControllerClientProxy_ReturnContentTypes_Tests.cs +++ b/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/RegularTestControllerClientProxy_ReturnContentTypes_Tests.cs @@ -57,8 +57,6 @@ public class RegularTestControllerClientProxy_ReturnContentTypes_Tests : AbpHttp [Fact] public async Task GetProducesJsonNullStringAsync_Should_Not_Return_Literal_Null() { - // Server returns JSON `null` body (4 chars). The unwrap MUST NOT pass through "null" literal — - // it should produce empty/null on the client side instead. var result = await _controller.GetProducesJsonNullStringAsync(); result.ShouldNotBe("null"); (result == null || result == string.Empty).ShouldBeTrue(); @@ -67,8 +65,6 @@ public class RegularTestControllerClientProxy_ReturnContentTypes_Tests : AbpHttp [Fact] public async Task GetEscapedStringAsync_Should_Decode_Escaped_Characters() { - // Server JSON-encodes the string with escapes: "a\"b\\c\nd" - // Without unwrap fix client would receive the raw JSON string including escapes. var result = await _controller.GetEscapedStringAsync(); result.ShouldBe("a\"b\\c\nd"); } @@ -83,12 +79,17 @@ public class RegularTestControllerClientProxy_ReturnContentTypes_Tests : AbpHttp content.FileName.ShouldBe("icon.bin"); } + [Fact] + public async Task GetReferenceTypeObjectAsync_Should_Not_Be_Wrapped_As_RemoteStreamContent() + { + var result = await _controller.GetReferenceTypeObjectAsync(); + result.ShouldNotBeNull(); + result.ShouldNotBeAssignableTo(); + } + [Fact] public async Task GetByteArrayAsync_Should_Round_Trip_Bytes() { - // byte[] is not IRemoteStreamContent; goes through default JSON path - // (server JSON-encodes as base64). Ensures our Accept logic didn't break - // the existing non-stream binary case. var bytes = await _controller.GetByteArrayAsync(); bytes.ShouldBe(new byte[] { 1, 2, 3, 4 }); } diff --git a/framework/test/Volo.Abp.Http.Tests/Volo/Abp/Http/Modeling/ReturnValueApiDescriptionModel_Tests.cs b/framework/test/Volo.Abp.Http.Tests/Volo/Abp/Http/Modeling/ReturnValueApiDescriptionModel_Tests.cs index c95f5d9be3..d3c3197937 100644 --- a/framework/test/Volo.Abp.Http.Tests/Volo/Abp/Http/Modeling/ReturnValueApiDescriptionModel_Tests.cs +++ b/framework/test/Volo.Abp.Http.Tests/Volo/Abp/Http/Modeling/ReturnValueApiDescriptionModel_Tests.cs @@ -49,10 +49,10 @@ public class ReturnValueApiDescriptionModel_IsRemoteStream_Tests } [Fact] - public void Custom_Subclass_Of_IRemoteStreamContent_Should_Be_True() + public void Custom_Subclass_Of_IRemoteStreamContent_Should_Be_False() { var model = ReturnValueApiDescriptionModel.Create(typeof(MyCustomStreamContent)); - model.IsRemoteStream.ShouldBeTrue(); + model.IsRemoteStream.ShouldBeFalse(); } [Fact] @@ -63,45 +63,45 @@ public class ReturnValueApiDescriptionModel_IsRemoteStream_Tests } [Fact] - public void Task_Of_Custom_Stream_Subclass_Should_Be_True_After_UnwrapTask() + public void Task_Of_Custom_Stream_Subclass_Should_Be_False_After_UnwrapTask() { var model = ReturnValueApiDescriptionModel.Create(typeof(Task)); - model.IsRemoteStream.ShouldBeTrue(); + model.IsRemoteStream.ShouldBeFalse(); } [Fact] - public void IRemoteStreamContent_Array_Should_Be_True() + public void IRemoteStreamContent_Array_Should_Be_False() { var model = ReturnValueApiDescriptionModel.Create(typeof(IRemoteStreamContent[])); - model.IsRemoteStream.ShouldBeTrue(); + model.IsRemoteStream.ShouldBeFalse(); } [Fact] - public void Concrete_RemoteStreamContent_Array_Should_Be_True() + public void Concrete_RemoteStreamContent_Array_Should_Be_False() { var model = ReturnValueApiDescriptionModel.Create(typeof(RemoteStreamContent[])); - model.IsRemoteStream.ShouldBeTrue(); + model.IsRemoteStream.ShouldBeFalse(); } [Fact] - public void List_Of_IRemoteStreamContent_Should_Be_True() + public void List_Of_IRemoteStreamContent_Should_Be_False() { var model = ReturnValueApiDescriptionModel.Create(typeof(List)); - model.IsRemoteStream.ShouldBeTrue(); + model.IsRemoteStream.ShouldBeFalse(); } [Fact] - public void IEnumerable_Of_IRemoteStreamContent_Should_Be_True() + public void IEnumerable_Of_IRemoteStreamContent_Should_Be_False() { var model = ReturnValueApiDescriptionModel.Create(typeof(IEnumerable)); - model.IsRemoteStream.ShouldBeTrue(); + model.IsRemoteStream.ShouldBeFalse(); } [Fact] - public void IReadOnlyCollection_Of_IRemoteStreamContent_Should_Be_True() + public void IReadOnlyCollection_Of_IRemoteStreamContent_Should_Be_False() { var model = ReturnValueApiDescriptionModel.Create(typeof(IReadOnlyCollection)); - model.IsRemoteStream.ShouldBeTrue(); + model.IsRemoteStream.ShouldBeFalse(); } [Fact] @@ -128,9 +128,6 @@ public class ReturnValueApiDescriptionModel_IsRemoteStream_Tests [Fact] public void Dto_Containing_IRemoteStreamContent_Property_Should_Be_False() { - // ABP design constraint: IRemoteStreamContent only works as the DIRECT endpoint return type. - // When nested inside a DTO, the server JSON-serializes the DTO and the stream metadata only — - // the binary payload is lost. So the proxy should NOT use blob mode for this case. var model = ReturnValueApiDescriptionModel.Create(typeof(DtoWithStream)); model.IsRemoteStream.ShouldBeFalse(); } @@ -145,7 +142,6 @@ public class ReturnValueApiDescriptionModel_IsRemoteStream_Tests [Fact] public void Byte_Array_Should_Be_False() { - // byte[] is serialized as base64 JSON by ABP, not as binary stream. var model = ReturnValueApiDescriptionModel.Create(typeof(byte[])); model.IsRemoteStream.ShouldBeFalse(); } @@ -189,7 +185,6 @@ public class ReturnValueApiDescriptionModel_BackwardsCompat_Tests [Fact] public void Deserializing_Json_Without_ContentTypes_Field_Should_Leave_It_Null() { - // Old backend JSON (no contentTypes field) read by new client. var json = """ { "type": "System.String", @@ -238,7 +233,6 @@ public class ReturnValueApiDescriptionModel_BackwardsCompat_Tests var model = ReturnValueApiDescriptionModel.Create(typeof(string)); var json = System.Text.Json.JsonSerializer.Serialize(model); - // Either "contentTypes":null or omitted — both are acceptable for old clients. var deserialized = System.Text.Json.JsonSerializer.Deserialize( json, new System.Text.Json.JsonSerializerOptions { PropertyNameCaseInsensitive = true });