From e4077d55690f69188b9c43df2aaaaf6ec121f8d4 Mon Sep 17 00:00:00 2001 From: Alexandru Bagu Date: Fri, 28 May 2021 14:44:01 +0300 Subject: [PATCH 1/8] 1. allow the usage of IRemoteContentStream or RemoteContentStream when the incoming stream is not a form content format 2. remove all rewinds related to streams; the user is the one who must ensure his streams are at the correct position when calling the api; stream rewind would exclude the ability of sending partial streams (at least skippable streams, because the tail cannot be limitted in .net core) 3. RemoteStreamContent now must get the content type associated with the stream in its constructor; it may also receive the length of the stream in a read-only form (this is because some stream classes cannot provide the length and it is provided via http headers) 4. IRemoteStreamContent should implement IDisposable to allow the auto clean up of streams (example: FileStream) --- .../AbpRemoteStreamContentModelBinder.cs | 9 +-- .../RemoteStreamContentOutputFormatter.cs | 10 +-- .../Volo/Abp/Content/IRemoteStreamContent.cs | 5 +- .../Volo/Abp/Content/RemoteStreamContent.cs | 23 ++++-- .../Volo/Abp/Extensions/StreamExtensions.cs | 29 ++++++++ .../DynamicHttpProxyInterceptor.cs | 5 +- .../DynamicProxying/RequestPayloadBuilder.cs | 14 ++-- .../RemoteStreamContentTestController.cs | 17 +++-- ...RemoteStreamContentTestController_Tests.cs | 26 ++++++- .../PersonAppServiceClientProxy_Tests.cs | 72 ++++++++----------- .../TestApp/Application/PeopleAppService.cs | 6 +- 11 files changed, 130 insertions(+), 86 deletions(-) create mode 100644 framework/src/Volo.Abp.Core/Volo/Abp/Extensions/StreamExtensions.cs diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/AbpRemoteStreamContentModelBinder.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/AbpRemoteStreamContentModelBinder.cs index da58641d7e..e4f3e46a77 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/AbpRemoteStreamContentModelBinder.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/AbpRemoteStreamContentModelBinder.cs @@ -108,13 +108,14 @@ namespace Volo.Abp.AspNetCore.Mvc.ContentFormatters if (file.Name.Equals(modelName, StringComparison.OrdinalIgnoreCase)) { - postedFiles.Add(new RemoteStreamContent(file.OpenReadStream()) - { - ContentType = file.ContentType - }); + postedFiles.Add(new RemoteStreamContent(file.OpenReadStream(), file.ContentType, file.Length)); } } } + else if (bindingContext.IsTopLevelObject) + { + postedFiles.Add(new RemoteStreamContent(request.Body, request.ContentType, request.ContentLength)); + } } } } diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentOutputFormatter.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentOutputFormatter.cs index 188306227a..0edc91d30a 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentOutputFormatter.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentOutputFormatter.cs @@ -1,4 +1,5 @@ using System; +using System.Buffers; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.Net.Http.Headers; @@ -27,13 +28,8 @@ namespace Volo.Abp.AspNetCore.Mvc.ContentFormatters context.HttpContext.Response.ContentType = remoteStream.ContentType; using (var stream = remoteStream.GetStream()) - { - if (stream.CanSeek) - { - stream.Position = 0; - } - - await stream.CopyToAsync(context.HttpContext.Response.Body); + { + await stream.CopyToAsync(context.HttpContext.Response.Body); } } } diff --git a/framework/src/Volo.Abp.Core/Volo/Abp/Content/IRemoteStreamContent.cs b/framework/src/Volo.Abp.Core/Volo/Abp/Content/IRemoteStreamContent.cs index bca259d422..4805983287 100644 --- a/framework/src/Volo.Abp.Core/Volo/Abp/Content/IRemoteStreamContent.cs +++ b/framework/src/Volo.Abp.Core/Volo/Abp/Content/IRemoteStreamContent.cs @@ -1,8 +1,9 @@ -using System.IO; +using System; +using System.IO; namespace Volo.Abp.Content { - public interface IRemoteStreamContent + public interface IRemoteStreamContent : IDisposable { string ContentType { get; } diff --git a/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs b/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs index f217101cea..fc30b93f7d 100644 --- a/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs +++ b/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs @@ -4,20 +4,31 @@ namespace Volo.Abp.Content { public class RemoteStreamContent : IRemoteStreamContent { - private readonly Stream _stream; - - public RemoteStreamContent(Stream stream) + private readonly Stream _stream; + private readonly string _contentType; + private readonly long? _length; + private readonly bool _leaveOpen; + + public RemoteStreamContent(Stream stream, string contentType, long? readOnlylength = null, bool leaveOpen = false) { _stream = stream; + _contentType = contentType; + _length = readOnlylength ?? (stream.GetNullableLength() - stream.GetNullablePosition()); + _leaveOpen = leaveOpen; } - public virtual string ContentType { get; set; } - - public virtual long? ContentLength => _stream.Length; + public virtual string ContentType => _contentType; + public virtual long? ContentLength => _length; public virtual Stream GetStream() { return _stream; } + + public virtual void Dispose() + { + if (!_leaveOpen) + _stream?.Dispose(); + } } } diff --git a/framework/src/Volo.Abp.Core/Volo/Abp/Extensions/StreamExtensions.cs b/framework/src/Volo.Abp.Core/Volo/Abp/Extensions/StreamExtensions.cs new file mode 100644 index 0000000000..0501e92d56 --- /dev/null +++ b/framework/src/Volo.Abp.Core/Volo/Abp/Extensions/StreamExtensions.cs @@ -0,0 +1,29 @@ +using System.IO; + +public static class StreamExtensions +{ + public static long? GetNullableLength(this Stream stream) + { + try + { + return stream?.Length; + } + catch + { + /*some stream classes throw exceptions when accessing Length because they do not have access to such information */ + return null; + } + } + public static long? GetNullablePosition(this Stream stream) + { + try + { + return stream?.Position; + } + catch + { + /*some stream classes throw exceptions when accessing Position because they do not have access to such information */ + return null; + } + } +} diff --git a/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/DynamicHttpProxyInterceptor.cs b/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/DynamicHttpProxyInterceptor.cs index dd96ad98a2..78773cc364 100644 --- a/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/DynamicHttpProxyInterceptor.cs +++ b/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/DynamicHttpProxyInterceptor.cs @@ -112,10 +112,7 @@ namespace Volo.Abp.Http.Client.DynamicProxying /* returning a class that holds a reference to response * content just to be sure that GC does not dispose of * it before we finish doing our work with the stream */ - return (T)(object)new RemoteStreamContent(await responseContent.ReadAsStreamAsync()) - { - ContentType = responseContent.Headers.ContentType?.ToString() - }; + return (T)(object)new RemoteStreamContent(await responseContent.ReadAsStreamAsync(), responseContent.Headers.ContentType?.ToString(), responseContent.Headers.ContentLength); } var stringContent = await responseContent.ReadAsStringAsync(); diff --git a/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/RequestPayloadBuilder.cs b/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/RequestPayloadBuilder.cs index 1096fa971e..47b2558f4c 100644 --- a/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/RequestPayloadBuilder.cs +++ b/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/RequestPayloadBuilder.cs @@ -82,15 +82,12 @@ namespace Volo.Abp.Http.Client.DynamicProxying if (value is IRemoteStreamContent remoteStreamContent) { var stream = remoteStreamContent.GetStream(); - if (stream.CanSeek) - { - stream.Position = 0; - } var streamContent = new StreamContent(stream); if (!remoteStreamContent.ContentType.IsNullOrWhiteSpace()) { streamContent.Headers.ContentType = new MediaTypeHeaderValue(remoteStreamContent.ContentType); - } + } + streamContent.Headers.ContentLength = stream.GetNullableLength() - stream.GetNullablePosition(); formData.Add(streamContent, parameter.Name, parameter.Name); } else if (value is IEnumerable remoteStreamContents) @@ -98,15 +95,12 @@ namespace Volo.Abp.Http.Client.DynamicProxying foreach (var content in remoteStreamContents) { var stream = content.GetStream(); - if (stream.CanSeek) - { - stream.Position = 0; - } var streamContent = new StreamContent(stream); if (!content.ContentType.IsNullOrWhiteSpace()) { streamContent.Headers.ContentType = new MediaTypeHeaderValue(content.ContentType); - } + } + streamContent.Headers.ContentLength = stream.GetNullableLength() - stream.GetNullablePosition(); formData.Add(streamContent, parameter.Name, parameter.Name); } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentTestController.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentTestController.cs index e74c9d52e8..de7cc8d5cc 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentTestController.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentTestController.cs @@ -16,11 +16,8 @@ namespace Volo.Abp.AspNetCore.Mvc.ContentFormatters { var memoryStream = new MemoryStream(); await memoryStream.WriteAsync(Encoding.UTF8.GetBytes("DownloadAsync")); - - return new RemoteStreamContent(memoryStream) - { - ContentType = "application/rtf" - }; + memoryStream.Position = 0; + return new RemoteStreamContent(memoryStream, "application/rtf"); } [HttpPost] @@ -32,5 +29,15 @@ namespace Volo.Abp.AspNetCore.Mvc.ContentFormatters return await reader.ReadToEndAsync() + ":" + file.ContentType; } } + + [HttpPost] + [Route("Upload-Raw")] + public async Task UploadRawAsync(IRemoteStreamContent file) + { + using (var reader = new StreamReader(file.GetStream())) + { + return await reader.ReadToEndAsync() + ":" + file.ContentType; + } + } } } diff --git a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentTestController_Tests.cs b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentTestController_Tests.cs index 12745da732..a901c131a2 100644 --- a/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentTestController_Tests.cs +++ b/framework/test/Volo.Abp.AspNetCore.Mvc.Tests/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentTestController_Tests.cs @@ -16,8 +16,8 @@ namespace Volo.Abp.AspNetCore.Mvc.ContentFormatters var result = await GetResponseAsync("/api/remote-stream-content-test/download"); result.Content.Headers.ContentType?.ToString().ShouldBe("application/rtf"); (await result.Content.ReadAsStringAsync()).ShouldBe("DownloadAsync"); - } - + } + [Fact] public async Task UploadAsync() { @@ -30,12 +30,32 @@ namespace Volo.Abp.AspNetCore.Mvc.ContentFormatters var streamContent = new StreamContent(memoryStream); streamContent.Headers.ContentType = new MediaTypeHeaderValue("application/rtf"); - requestMessage.Content = new MultipartFormDataContent {{streamContent, "file", "file"}}; + requestMessage.Content = new MultipartFormDataContent { { streamContent, "file", "file" } }; var response = await Client.SendAsync(requestMessage); (await response.Content.ReadAsStringAsync()).ShouldBe("UploadAsync:application/rtf"); } } + + [Fact] + public async Task UploadRawAsync() + { + using (var requestMessage = new HttpRequestMessage(HttpMethod.Post, "/api/remote-stream-content-test/upload-raw")) + { + var memoryStream = new MemoryStream(); + var text = @"{ ""hello"": ""world"" }"; + await memoryStream.WriteAsync(Encoding.UTF8.GetBytes(text)); + memoryStream.Position = 0; + + var streamContent = new StreamContent(memoryStream); + streamContent.Headers.ContentType = new MediaTypeHeaderValue("application/json"); + + requestMessage.Content = streamContent; + + var response = await Client.SendAsync(requestMessage); + (await response.Content.ReadAsStringAsync()).ShouldBe($"{text}:application/json"); + } + } } } diff --git a/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/PersonAppServiceClientProxy_Tests.cs b/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/PersonAppServiceClientProxy_Tests.cs index 026373d568..9f172fcfd1 100644 --- a/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/PersonAppServiceClientProxy_Tests.cs +++ b/framework/test/Volo.Abp.Http.Client.Tests/Volo/Abp/Http/DynamicProxying/PersonAppServiceClientProxy_Tests.cs @@ -49,7 +49,7 @@ namespace Volo.Abp.Http.DynamicProxying { var people = await _peopleAppService.GetListAsync(new PagedAndSortedResultRequestDto()); people.TotalCount.ShouldBeGreaterThan(0); - people.Items.Count.ShouldBe((int) people.TotalCount); + people.Items.Count.ShouldBe((int)people.TotalCount); } [Fact] @@ -62,7 +62,7 @@ namespace Volo.Abp.Http.DynamicProxying { id1, id2 - }, new[] {"name1", "name2"}); + }, new[] { "name1", "name2" }); @params.ShouldContain(id1.ToString("N")); @params.ShouldContain(id2.ToString("N")); @@ -86,11 +86,11 @@ namespace Volo.Abp.Http.DynamicProxying { var uniquePersonName = Guid.NewGuid().ToString(); - var person = await _peopleAppService.CreateAsync(new PersonDto - { - Name = uniquePersonName, - Age = 42 - } + var person = await _peopleAppService.CreateAsync(new PersonDto + { + Name = uniquePersonName, + Age = 42 + } ); person.ShouldNotBeNull(); @@ -107,10 +107,10 @@ namespace Volo.Abp.Http.DynamicProxying { await Assert.ThrowsAsync(async () => { - var person = await _peopleAppService.CreateAsync(new PersonDto - { - Age = 42 - } + var person = await _peopleAppService.CreateAsync(new PersonDto + { + Age = 42 + } ); }); } @@ -194,10 +194,20 @@ namespace Volo.Abp.Http.DynamicProxying var memoryStream = new MemoryStream(); await memoryStream.WriteAsync(Encoding.UTF8.GetBytes("UploadAsync")); memoryStream.Position = 0; - var result = await _peopleAppService.UploadAsync(new RemoteStreamContent(memoryStream) - { - ContentType = "application/rtf" - }); + var result = await _peopleAppService.UploadAsync(new RemoteStreamContent(memoryStream, "application/rtf")); + result.ShouldBe("UploadAsync:application/rtf"); + } + + [Fact] + public async Task UploadPartialAsync() + { + var memoryStream = new MemoryStream(); + var rawData = new byte[16]; + var text = Encoding.UTF8.GetBytes("UploadAsync"); + await memoryStream.WriteAsync(rawData); + await memoryStream.WriteAsync(text); + memoryStream.Position = rawData.Length; + var result = await _peopleAppService.UploadAsync(new RemoteStreamContent(memoryStream, "application/rtf")); result.ShouldBe("UploadAsync:application/rtf"); } @@ -214,15 +224,8 @@ namespace Volo.Abp.Http.DynamicProxying var result = await _peopleAppService.UploadMultipleAsync(new List() { - new RemoteStreamContent(memoryStream) - { - ContentType = "application/rtf" - }, - - new RemoteStreamContent(memoryStream2) - { - ContentType = "application/rtf2" - } + new RemoteStreamContent(memoryStream, "application/rtf"), + new RemoteStreamContent(memoryStream2, "application/rtf2") }); result.ShouldBe("File1:application/rtfFile2:application/rtf2"); } @@ -236,10 +239,7 @@ namespace Volo.Abp.Http.DynamicProxying var result = await _peopleAppService.CreateFileAsync(new CreateFileInput() { Name = "123.rtf", - Content = new RemoteStreamContent(memoryStream) - { - ContentType = "application/rtf" - } + Content = new RemoteStreamContent(memoryStream, "application/rtf") }); result.ShouldBe("123.rtf:CreateFileAsync:application/rtf"); } @@ -264,23 +264,13 @@ namespace Volo.Abp.Http.DynamicProxying Name = "123.rtf", Contents = new List() { - new RemoteStreamContent(memoryStream) - { - ContentType = "application/rtf" - }, - - new RemoteStreamContent(memoryStream2) - { - ContentType = "application/rtf2" - } + new RemoteStreamContent(memoryStream, "application/rtf"), + new RemoteStreamContent(memoryStream2, "application/rtf2"), }, Inner = new CreateFileInput() { Name = "789.rtf", - Content = new RemoteStreamContent(memoryStream3) - { - ContentType = "application/rtf3" - } + Content = new RemoteStreamContent(memoryStream3, "application/rtf3") } }); result.ShouldBe("123.rtf:File1:application/rtf123.rtf:File2:application/rtf2789.rtf:File3:application/rtf3"); diff --git a/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Application/PeopleAppService.cs b/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Application/PeopleAppService.cs index edd6a3b93f..6841882ce6 100644 --- a/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Application/PeopleAppService.cs +++ b/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Application/PeopleAppService.cs @@ -72,11 +72,9 @@ namespace Volo.Abp.TestApp.Application { var memoryStream = new MemoryStream(); await memoryStream.WriteAsync(Encoding.UTF8.GetBytes("DownloadAsync")); + memoryStream.Position = 0; - return new RemoteStreamContent(memoryStream) - { - ContentType = "application/rtf" - }; + return new RemoteStreamContent(memoryStream, "application/rtf"); } public async Task UploadAsync(IRemoteStreamContent streamContent) From 990866f1a3527e654d30398362447e2d9ff768e5 Mon Sep 17 00:00:00 2001 From: Alexandru Bagu Date: Fri, 28 May 2021 16:47:33 +0300 Subject: [PATCH 2/8] fix build with new api --- .../CmsKit/Admin/MediaDescriptors/CreateMediaInputStream.cs | 2 +- .../CmsKit/MediaDescriptors/MediaDescriptorAppService.cs | 5 +---- .../MediaDescriptors/MediaDescriptorAdminAppService_Tests.cs | 5 +---- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/modules/cms-kit/src/Volo.CmsKit.Admin.Application.Contracts/Volo/CmsKit/Admin/MediaDescriptors/CreateMediaInputStream.cs b/modules/cms-kit/src/Volo.CmsKit.Admin.Application.Contracts/Volo/CmsKit/Admin/MediaDescriptors/CreateMediaInputStream.cs index 0f66ab5fef..1c62622b68 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Admin.Application.Contracts/Volo/CmsKit/Admin/MediaDescriptors/CreateMediaInputStream.cs +++ b/modules/cms-kit/src/Volo.CmsKit.Admin.Application.Contracts/Volo/CmsKit/Admin/MediaDescriptors/CreateMediaInputStream.cs @@ -16,7 +16,7 @@ namespace Volo.CmsKit.Admin.MediaDescriptors [DynamicStringLength(typeof(MediaDescriptorConsts), nameof(MediaDescriptorConsts.MaxNameLength))] public string Name { get; set; } - public CreateMediaInputStream(Stream stream) : base(stream) + public CreateMediaInputStream(Stream stream, string contentType) : base(stream, contentType) { } } diff --git a/modules/cms-kit/src/Volo.CmsKit.Common.Application/Volo/CmsKit/MediaDescriptors/MediaDescriptorAppService.cs b/modules/cms-kit/src/Volo.CmsKit.Common.Application/Volo/CmsKit/MediaDescriptors/MediaDescriptorAppService.cs index 0068390312..366a86c0b0 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Common.Application/Volo/CmsKit/MediaDescriptors/MediaDescriptorAppService.cs +++ b/modules/cms-kit/src/Volo.CmsKit.Common.Application/Volo/CmsKit/MediaDescriptors/MediaDescriptorAppService.cs @@ -24,10 +24,7 @@ namespace Volo.CmsKit.MediaDescriptors var entity = await MediaDescriptorRepository.GetAsync(id); var stream = await MediaContainer.GetAsync(id.ToString()); - return new RemoteStreamContent(stream) - { - ContentType = entity.MimeType - }; + return new RemoteStreamContent(stream, entity.MimeType); } } } \ No newline at end of file diff --git a/modules/cms-kit/test/Volo.CmsKit.Application.Tests/MediaDescriptors/MediaDescriptorAdminAppService_Tests.cs b/modules/cms-kit/test/Volo.CmsKit.Application.Tests/MediaDescriptors/MediaDescriptorAdminAppService_Tests.cs index 38a8b472bd..898b58d437 100644 --- a/modules/cms-kit/test/Volo.CmsKit.Application.Tests/MediaDescriptors/MediaDescriptorAdminAppService_Tests.cs +++ b/modules/cms-kit/test/Volo.CmsKit.Application.Tests/MediaDescriptors/MediaDescriptorAdminAppService_Tests.cs @@ -35,10 +35,7 @@ namespace Volo.CmsKit.MediaDescriptors var media = await _mediaDescriptorAdminAppService.CreateAsync(_cmsKitTestData.Media_1_EntityType, new CreateMediaInputWithStream { Name = mediaName, - File = new RemoteStreamContent(stream) - { - ContentType = mediaType - } + File = new RemoteStreamContent(stream, mediaType) }); media.ShouldNotBeNull(); From 686d28dea7486e47794e9fb57181df85201e74fe Mon Sep 17 00:00:00 2001 From: maliming Date: Mon, 19 Jul 2021 18:05:37 +0800 Subject: [PATCH 3/8] Refactor. --- .../Volo/Abp/Content/IRemoteStreamContent.cs | 4 +-- .../Volo/Abp/Content/RemoteStreamContent.cs | 35 ++++++++++++------- .../Volo/Abp/Extensions/StreamExtensions.cs | 29 --------------- .../DynamicHttpProxyInterceptor.cs | 8 ++--- .../DynamicProxying/RequestPayloadBuilder.cs | 4 +-- .../MediaDescriptorAppService.cs | 4 +-- .../MediaDescriptorAdminAppService_Tests.cs | 8 ++--- 7 files changed, 36 insertions(+), 56 deletions(-) delete mode 100644 framework/src/Volo.Abp.Core/Volo/Abp/Extensions/StreamExtensions.cs diff --git a/framework/src/Volo.Abp.Core/Volo/Abp/Content/IRemoteStreamContent.cs b/framework/src/Volo.Abp.Core/Volo/Abp/Content/IRemoteStreamContent.cs index 4cabd5b5e0..ba564df18e 100644 --- a/framework/src/Volo.Abp.Core/Volo/Abp/Content/IRemoteStreamContent.cs +++ b/framework/src/Volo.Abp.Core/Volo/Abp/Content/IRemoteStreamContent.cs @@ -5,12 +5,12 @@ namespace Volo.Abp.Content { public interface IRemoteStreamContent : IDisposable { + string FileName { get; } + string ContentType { get; } long? ContentLength { get; } - string FileName { get; } - Stream GetStream(); } } diff --git a/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs b/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs index f2e69f49ad..f663066e34 100644 --- a/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs +++ b/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs @@ -5,22 +5,30 @@ namespace Volo.Abp.Content public class RemoteStreamContent : IRemoteStreamContent { private readonly Stream _stream; - private readonly string _fileName; - private readonly string _contentType; - private readonly long? _length; - private readonly bool _leaveOpen; + private readonly bool _disposeStream; + private bool _disposed; - public virtual string FileName => _fileName; - public virtual string ContentType => _contentType; - public virtual long? ContentLength => _length; + public virtual string FileName { get; } - public RemoteStreamContent(Stream stream, string fileName, string contentType = null, long? readOnlylength = null, bool leaveOpen = false) + public virtual string ContentType { get; } = "application/octet-stream"; + + public virtual long? ContentLength { get; } + + public RemoteStreamContent(Stream stream, bool disposeStream = true) { _stream = stream; - _fileName = fileName; - _contentType = contentType ?? "application/octet-stream"; - _length = readOnlylength ?? (stream.GetNullableLength() - stream.GetNullablePosition()); - _leaveOpen = leaveOpen; + _disposeStream = disposeStream; + } + + public RemoteStreamContent(Stream stream, string fileName, string contentType = null, long? readOnlyLength = null, bool disposeStream = true) + : this(stream, disposeStream) + { + FileName = fileName; + if (contentType != null) + { + ContentType = contentType; + } + ContentLength = readOnlyLength ?? (_stream.CanSeek ? _stream.Length - _stream.Position : null); } public virtual Stream GetStream() @@ -30,8 +38,9 @@ namespace Volo.Abp.Content public virtual void Dispose() { - if (!_leaveOpen) + if (!_disposed && _disposeStream) { + _disposed = true; _stream?.Dispose(); } } diff --git a/framework/src/Volo.Abp.Core/Volo/Abp/Extensions/StreamExtensions.cs b/framework/src/Volo.Abp.Core/Volo/Abp/Extensions/StreamExtensions.cs deleted file mode 100644 index 0501e92d56..0000000000 --- a/framework/src/Volo.Abp.Core/Volo/Abp/Extensions/StreamExtensions.cs +++ /dev/null @@ -1,29 +0,0 @@ -using System.IO; - -public static class StreamExtensions -{ - public static long? GetNullableLength(this Stream stream) - { - try - { - return stream?.Length; - } - catch - { - /*some stream classes throw exceptions when accessing Length because they do not have access to such information */ - return null; - } - } - public static long? GetNullablePosition(this Stream stream) - { - try - { - return stream?.Position; - } - catch - { - /*some stream classes throw exceptions when accessing Position because they do not have access to such information */ - return null; - } - } -} diff --git a/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/DynamicHttpProxyInterceptor.cs b/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/DynamicHttpProxyInterceptor.cs index 9f41142e15..a83d25910a 100644 --- a/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/DynamicHttpProxyInterceptor.cs +++ b/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/DynamicHttpProxyInterceptor.cs @@ -113,11 +113,11 @@ namespace Volo.Abp.Http.Client.DynamicProxying /* returning a class that holds a reference to response * content just to be sure that GC does not dispose of * it before we finish doing our work with the stream */ - return (T)(object)new RemoteStreamContent( - await responseContent.ReadAsStreamAsync(), + return (T) (object) new RemoteStreamContent( + await responseContent.ReadAsStreamAsync(), responseContent.Headers?.ContentDisposition?.FileNameStar ?? RemoveQuotes(responseContent.Headers?.ContentDisposition?.FileName).ToString(), - responseContent.Headers.ContentType?.ToString(), - responseContent.Headers.ContentLength); + responseContent.Headers?.ContentType?.ToString(), + responseContent.Headers?.ContentLength); } var stringContent = await responseContent.ReadAsStringAsync(); diff --git a/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/RequestPayloadBuilder.cs b/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/RequestPayloadBuilder.cs index 7893073442..d83b319b28 100644 --- a/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/RequestPayloadBuilder.cs +++ b/framework/src/Volo.Abp.Http.Client/Volo/Abp/Http/Client/DynamicProxying/RequestPayloadBuilder.cs @@ -87,7 +87,7 @@ namespace Volo.Abp.Http.Client.DynamicProxying { streamContent.Headers.ContentType = new MediaTypeHeaderValue(remoteStreamContent.ContentType); } - streamContent.Headers.ContentLength = stream.GetNullableLength() - stream.GetNullablePosition(); + streamContent.Headers.ContentLength = remoteStreamContent.ContentLength; formData.Add(streamContent, parameter.Name, remoteStreamContent.FileName ?? parameter.Name); } else if (value is IEnumerable remoteStreamContents) @@ -100,7 +100,7 @@ namespace Volo.Abp.Http.Client.DynamicProxying { streamContent.Headers.ContentType = new MediaTypeHeaderValue(content.ContentType); } - streamContent.Headers.ContentLength = stream.GetNullableLength() - stream.GetNullablePosition(); + streamContent.Headers.ContentLength = content.ContentLength; formData.Add(streamContent, parameter.Name, content.FileName ?? parameter.Name); } } diff --git a/modules/cms-kit/src/Volo.CmsKit.Common.Application/Volo/CmsKit/MediaDescriptors/MediaDescriptorAppService.cs b/modules/cms-kit/src/Volo.CmsKit.Common.Application/Volo/CmsKit/MediaDescriptors/MediaDescriptorAppService.cs index 366a86c0b0..99700630e7 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Common.Application/Volo/CmsKit/MediaDescriptors/MediaDescriptorAppService.cs +++ b/modules/cms-kit/src/Volo.CmsKit.Common.Application/Volo/CmsKit/MediaDescriptors/MediaDescriptorAppService.cs @@ -24,7 +24,7 @@ namespace Volo.CmsKit.MediaDescriptors var entity = await MediaDescriptorRepository.GetAsync(id); var stream = await MediaContainer.GetAsync(id.ToString()); - return new RemoteStreamContent(stream, entity.MimeType); + return new RemoteStreamContent(stream, entity.Name, entity.MimeType); } } -} \ No newline at end of file +} diff --git a/modules/cms-kit/test/Volo.CmsKit.Application.Tests/MediaDescriptors/MediaDescriptorAdminAppService_Tests.cs b/modules/cms-kit/test/Volo.CmsKit.Application.Tests/MediaDescriptors/MediaDescriptorAdminAppService_Tests.cs index 898b58d437..2559d6ab90 100644 --- a/modules/cms-kit/test/Volo.CmsKit.Application.Tests/MediaDescriptors/MediaDescriptorAdminAppService_Tests.cs +++ b/modules/cms-kit/test/Volo.CmsKit.Application.Tests/MediaDescriptors/MediaDescriptorAdminAppService_Tests.cs @@ -35,12 +35,12 @@ namespace Volo.CmsKit.MediaDescriptors var media = await _mediaDescriptorAdminAppService.CreateAsync(_cmsKitTestData.Media_1_EntityType, new CreateMediaInputWithStream { Name = mediaName, - File = new RemoteStreamContent(stream, mediaType) + File = new RemoteStreamContent(stream, mediaName, mediaType) }); - + media.ShouldNotBeNull(); } - + [Fact] public async Task Should_Delete_Media() { @@ -49,4 +49,4 @@ namespace Volo.CmsKit.MediaDescriptors (await _mediaDescriptorRepository.FindAsync(_cmsKitTestData.Media_1_Id)).ShouldBeNull(); } } -} \ No newline at end of file +} From 02e5c7f47f6668277c097d00970bd53633104f82 Mon Sep 17 00:00:00 2001 From: maliming Date: Mon, 19 Jul 2021 21:38:15 +0800 Subject: [PATCH 4/8] Make fileName optional. --- .../src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs b/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs index f663066e34..faab2df9fc 100644 --- a/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs +++ b/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs @@ -20,7 +20,7 @@ namespace Volo.Abp.Content _disposeStream = disposeStream; } - public RemoteStreamContent(Stream stream, string fileName, string contentType = null, long? readOnlyLength = null, bool disposeStream = true) + public RemoteStreamContent(Stream stream, string fileName = null, string contentType = null, long? readOnlyLength = null, bool disposeStream = true) : this(stream, disposeStream) { FileName = fileName; From 979538ecaa1504af20134bb5d545897bbed23ce8 Mon Sep 17 00:00:00 2001 From: maliming Date: Mon, 19 Jul 2021 21:50:45 +0800 Subject: [PATCH 5/8] Only keep one ctor. --- .../Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs b/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs index faab2df9fc..4b301a3ad0 100644 --- a/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs +++ b/framework/src/Volo.Abp.Core/Volo/Abp/Content/RemoteStreamContent.cs @@ -14,21 +14,17 @@ namespace Volo.Abp.Content public virtual long? ContentLength { get; } - public RemoteStreamContent(Stream stream, bool disposeStream = true) + public RemoteStreamContent(Stream stream, string fileName = null, string contentType = null, long? readOnlyLength = null, bool disposeStream = true) { _stream = stream; - _disposeStream = disposeStream; - } - public RemoteStreamContent(Stream stream, string fileName = null, string contentType = null, long? readOnlyLength = null, bool disposeStream = true) - : this(stream, disposeStream) - { FileName = fileName; if (contentType != null) { ContentType = contentType; } ContentLength = readOnlyLength ?? (_stream.CanSeek ? _stream.Length - _stream.Position : null); + _disposeStream = disposeStream; } public virtual Stream GetStream() From ec22181ba6d25d403b5d555799086c4085c96f58 Mon Sep 17 00:00:00 2001 From: maliming Date: Tue, 20 Jul 2021 15:05:44 +0800 Subject: [PATCH 6/8] dispose of the remoteStream object. --- .../RemoteStreamContentOutputFormatter.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentOutputFormatter.cs b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentOutputFormatter.cs index 26dd0b0999..50d65ce02d 100644 --- a/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentOutputFormatter.cs +++ b/framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/ContentFormatters/RemoteStreamContentOutputFormatter.cs @@ -1,5 +1,4 @@ using System; -using System.Buffers; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.Net.Http.Headers; @@ -34,9 +33,9 @@ namespace Volo.Abp.AspNetCore.Mvc.ContentFormatters context.HttpContext.Response.Headers[HeaderNames.ContentDisposition] = contentDisposition.ToString(); } - using (var stream = remoteStream.GetStream()) - { - await stream.CopyToAsync(context.HttpContext.Response.Body); + using (remoteStream) + { + await remoteStream.GetStream().CopyToAsync(context.HttpContext.Response.Body); } } } From 8f5a3ff2abcab3fa53463b284a762397c5d8947b Mon Sep 17 00:00:00 2001 From: maliming Date: Tue, 20 Jul 2021 16:39:58 +0800 Subject: [PATCH 7/8] Update MediaDescriptorAdminAppService. --- .../CreateMediaInputStream.cs | 23 ------------------- .../MediaDescriptorAdminAppService.cs | 8 +++---- 2 files changed, 4 insertions(+), 27 deletions(-) delete mode 100644 modules/cms-kit/src/Volo.CmsKit.Admin.Application.Contracts/Volo/CmsKit/Admin/MediaDescriptors/CreateMediaInputStream.cs diff --git a/modules/cms-kit/src/Volo.CmsKit.Admin.Application.Contracts/Volo/CmsKit/Admin/MediaDescriptors/CreateMediaInputStream.cs b/modules/cms-kit/src/Volo.CmsKit.Admin.Application.Contracts/Volo/CmsKit/Admin/MediaDescriptors/CreateMediaInputStream.cs deleted file mode 100644 index 1c62622b68..0000000000 --- a/modules/cms-kit/src/Volo.CmsKit.Admin.Application.Contracts/Volo/CmsKit/Admin/MediaDescriptors/CreateMediaInputStream.cs +++ /dev/null @@ -1,23 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using System.IO; -using Volo.Abp.Content; -using Volo.Abp.Validation; -using Volo.CmsKit.MediaDescriptors; - -namespace Volo.CmsKit.Admin.MediaDescriptors -{ - public class CreateMediaInputStream : RemoteStreamContent - { - [Required] - [DynamicStringLength(typeof(MediaDescriptorConsts), nameof(MediaDescriptorConsts.MaxEntityTypeLength))] - public string EntityType { get; set; } - - [Required] - [DynamicStringLength(typeof(MediaDescriptorConsts), nameof(MediaDescriptorConsts.MaxNameLength))] - public string Name { get; set; } - - public CreateMediaInputStream(Stream stream, string contentType) : base(stream, contentType) - { - } - } -} \ No newline at end of file diff --git a/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs b/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs index fa799cb65b..c9bdd6d18a 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs +++ b/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs @@ -18,7 +18,7 @@ namespace Volo.CmsKit.Admin.MediaDescriptors public MediaDescriptorAdminAppService( IBlobContainer mediaContainer, IMediaDescriptorRepository mediaDescriptorRepository, - MediaDescriptorManager mediaDescriptorManager, + MediaDescriptorManager mediaDescriptorManager, IMediaDescriptorDefinitionStore mediaDescriptorDefinitionStore) { MediaContainer = mediaContainer; @@ -34,11 +34,11 @@ namespace Volo.CmsKit.Admin.MediaDescriptors /* TODO: Shouldn't CreatePolicies be a dictionary and we check for inputStream.EntityType? */ await CheckAnyOfPoliciesAsync(definition.CreatePolicies); - using (var stream = inputStream.File.GetStream()) + using (var file = inputStream.File) { var newEntity = await MediaDescriptorManager.CreateAsync(entityType, inputStream.Name, inputStream.File.ContentType, inputStream.File.ContentLength ?? 0); - await MediaContainer.SaveAsync(newEntity.Id.ToString(), stream); + await MediaContainer.SaveAsync(newEntity.Id.ToString(), file.GetStream()); await MediaDescriptorRepository.InsertAsync(newEntity); return ObjectMapper.Map(newEntity); @@ -58,4 +58,4 @@ namespace Volo.CmsKit.Admin.MediaDescriptors await MediaDescriptorRepository.DeleteAsync(id); } } -} \ No newline at end of file +} From c15b9ad7ab849215ecbc240bf690dbe1cf349878 Mon Sep 17 00:00:00 2001 From: maliming Date: Tue, 20 Jul 2021 18:46:42 +0800 Subject: [PATCH 8/8] No need to dispose the inputStream.File. --- .../MediaDescriptorAdminAppService.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs b/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs index c9bdd6d18a..bab338e503 100644 --- a/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs +++ b/modules/cms-kit/src/Volo.CmsKit.Admin.Application/Volo/CmsKit/Admin/MediaDescriptors/MediaDescriptorAdminAppService.cs @@ -34,15 +34,12 @@ namespace Volo.CmsKit.Admin.MediaDescriptors /* TODO: Shouldn't CreatePolicies be a dictionary and we check for inputStream.EntityType? */ await CheckAnyOfPoliciesAsync(definition.CreatePolicies); - using (var file = inputStream.File) - { - var newEntity = await MediaDescriptorManager.CreateAsync(entityType, inputStream.Name, inputStream.File.ContentType, inputStream.File.ContentLength ?? 0); + var newEntity = await MediaDescriptorManager.CreateAsync(entityType, inputStream.Name, inputStream.File.ContentType, inputStream.File.ContentLength ?? 0); - await MediaContainer.SaveAsync(newEntity.Id.ToString(), file.GetStream()); - await MediaDescriptorRepository.InsertAsync(newEntity); + await MediaContainer.SaveAsync(newEntity.Id.ToString(), inputStream.File.GetStream()); + await MediaDescriptorRepository.InsertAsync(newEntity); - return ObjectMapper.Map(newEntity); - } + return ObjectMapper.Map(newEntity); } public virtual async Task DeleteAsync(Guid id)