From 43258f99c37d685fb97a25a7d0a6b4e7429d8139 Mon Sep 17 00:00:00 2001 From: David W <1511024+marabooy@users.noreply.github.com> Date: Mon, 23 Sep 2024 14:17:30 +0300 Subject: [PATCH] Update message writer and reader to ignore Message info from DI (#3058) Update message writer and reader to ignore Message info from DI as it is overwritten. --- buildandtest.yml | 7 +- nightly.yml | 5 +- .../ODataMessageReader.cs | 28 +++---- .../ODataMessageWriter.cs | 37 ++++----- .../MessageWriterConcurrencyTests.cs | 77 +++++++++++++++++++ 5 files changed, 109 insertions(+), 45 deletions(-) create mode 100644 test/FunctionalTests/Microsoft.OData.Core.Tests/MessageWriterConcurrencyTests.cs diff --git a/buildandtest.yml b/buildandtest.yml index 0796247d81..976e497b3f 100644 --- a/buildandtest.yml +++ b/buildandtest.yml @@ -20,11 +20,6 @@ steps: inputs: version: '8.x' -# .NET Core App 2.1 is required to run the ESRP code signing task -- task: UseDotNet@2 - inputs: - version: '2.1.x' - - task: DotNetCoreCLI@2 displayName: 'Build' inputs: @@ -48,4 +43,4 @@ steps: command: 'test' arguments: '--configuration $(buildConfiguration) --collect "Code coverage"' projects: | - $(Build.SourcesDirectory)\test\EndToEndTests\Tests\Client\Microsoft.OData.Client.E2E.Tests\Microsoft.OData.Client.E2E.Tests.csproj \ No newline at end of file + $(Build.SourcesDirectory)\test\EndToEndTests\Tests\Client\Microsoft.OData.Client.E2E.Tests\Microsoft.OData.Client.E2E.Tests.csproj diff --git a/nightly.yml b/nightly.yml index e4bd6bddca..f9b940ceba 100644 --- a/nightly.yml +++ b/nightly.yml @@ -11,7 +11,10 @@ BuildDropPath: '$(Build.ArtifactStagingDirectory)\$(nugetArtifactsDir)\sbom' - template: credscan.yml - + # .NET Core App 2.1 is required to run the ESRP code signing task + - task: UseDotNet@2 + inputs: + version: '2.1.x' # CodeSign - task: EsrpCodeSigning@1 displayName: 'ESRP CodeSign - OData' diff --git a/src/Microsoft.OData.Core/ODataMessageReader.cs b/src/Microsoft.OData.Core/ODataMessageReader.cs index 5b66f67bdb..00338fa61c 100644 --- a/src/Microsoft.OData.Core/ODataMessageReader.cs +++ b/src/Microsoft.OData.Core/ODataMessageReader.cs @@ -904,24 +904,18 @@ private ODataMessageInfo GetOrCreateMessageInfo(Stream messageStream, bool isAsy { if (this.messageInfo == null) { - if (this.serviceProvider == null) + this.messageInfo = new ODataMessageInfo { - this.messageInfo = new ODataMessageInfo(); - } - else - { - this.messageInfo = this.serviceProvider.GetRequiredService(); - } - - this.messageInfo.Encoding = this.encoding; - this.messageInfo.IsResponse = this.readingResponse; - this.messageInfo.IsAsync = isAsync; - this.messageInfo.MediaType = this.contentType; - this.messageInfo.Model = this.model; - this.messageInfo.PayloadUriConverter = this.payloadUriConverter; - this.messageInfo.ServiceProvider = this.serviceProvider; - this.messageInfo.MessageStream = messageStream; - this.messageInfo.PayloadKind = this.readerPayloadKind; + Encoding = this.encoding, + IsResponse = this.readingResponse, + IsAsync = isAsync, + MediaType = this.contentType, + Model = this.model, + PayloadUriConverter = this.payloadUriConverter, + ServiceProvider = this.serviceProvider, + MessageStream = messageStream, + PayloadKind = this.readerPayloadKind + }; } return this.messageInfo; diff --git a/src/Microsoft.OData.Core/ODataMessageWriter.cs b/src/Microsoft.OData.Core/ODataMessageWriter.cs index aa3ade2f22..fef522f3a2 100644 --- a/src/Microsoft.OData.Core/ODataMessageWriter.cs +++ b/src/Microsoft.OData.Core/ODataMessageWriter.cs @@ -19,9 +19,9 @@ namespace Microsoft.OData using Microsoft.OData.Metadata; #endregion Namespaces -/// -/// Writer class used to write all OData payloads (entries, resource sets, metadata documents, service documents, etc.). -/// + /// + /// Writer class used to write all OData payloads (entries, resource sets, metadata documents, service documents, etc.). + /// #if NETCOREAPP public sealed class ODataMessageWriter : IDisposable, IAsyncDisposable #else @@ -1111,7 +1111,7 @@ private ODataPayloadKind VerifyCanWriteValue(object value) // We cannot use ODataRawValueUtils.TryConvertPrimitiveToString for all cases since binary values are // converted into unencoded byte streams in the raw format // (as opposed to base64 encoded byte streams in the ODataRawValueUtils); see OIPI 2.2.6.4.1. - return value is byte[] ? ODataPayloadKind.BinaryValue : ODataPayloadKind.Value; + return value is byte[]? ODataPayloadKind.BinaryValue : ODataPayloadKind.Value; } /// @@ -1227,23 +1227,18 @@ private ODataMessageInfo GetOrCreateMessageInfo(Stream messageStream, bool isAsy { if (this.messageInfo == null) { - if (this.serviceProvider == null) - { - this.messageInfo = new ODataMessageInfo(); - } - else - { - this.messageInfo = this.serviceProvider.GetRequiredService(); - } - this.messageInfo.Encoding = this.encoding; - this.messageInfo.IsResponse = this.writingResponse; - this.messageInfo.IsAsync = isAsync; - this.messageInfo.MediaType = this.mediaType; - this.messageInfo.Model = this.model; - this.messageInfo.PayloadUriConverter = this.payloadUriConverter; - this.messageInfo.ServiceProvider = this.serviceProvider; - this.messageInfo.MessageStream = messageStream; + this.messageInfo = new ODataMessageInfo + { + Encoding = this.encoding, + IsResponse = this.writingResponse, + IsAsync = isAsync, + MediaType = this.mediaType, + Model = this.model, + PayloadUriConverter = this.payloadUriConverter, + ServiceProvider = this.serviceProvider, + MessageStream = messageStream + }; } return this.messageInfo; @@ -1321,7 +1316,7 @@ private async Task WriteToOutputAsync(ODataPayloadKind payload this.outputContext = await this.format.CreateOutputContextAsync( this.GetOrCreateMessageInfo(messageStream, true), this.settings).ConfigureAwait(false); - + return await writeFunc(this.outputContext).ConfigureAwait(false); } } diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/MessageWriterConcurrencyTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/MessageWriterConcurrencyTests.cs new file mode 100644 index 0000000000..c1154059f6 --- /dev/null +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/MessageWriterConcurrencyTests.cs @@ -0,0 +1,77 @@ +//--------------------------------------------------------------------- +// +// Copyright (C) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information. +// +//--------------------------------------------------------------------- + +using System.IO; +using System.Threading.Tasks; +using System; +using Xunit; +using Microsoft.Extensions.DependencyInjection; +using System.Linq; +using Microsoft.OData.Tests; + +namespace Microsoft.OData.Core.Tests +{ + public class MessageWriterConcurrencyTests + { + /// + /// Verifies that concurrent message writer does not interleave execution and isolates the underlying streams. + /// + /// A task for the asynchronous test + [Fact] + public async Task VerifyConcurrentResultsAreIsolatedAsync() + { + ServiceCollection services = new(); + services.AddDefaultODataServices(); + ServiceProvider serviceProvider = services.BuildServiceProvider(); + + var content1 = string.Concat(Enumerable.Repeat('A', 1000_000)); + var content2 = string.Concat(Enumerable.Repeat('B', 1000_000)); + for (int i = 0; i < 1000; i++) + { + var values = await Task.WhenAll([WritePayloadAsync(content1, serviceProvider), WritePayloadAsync(content2, serviceProvider)]); + Assert.Equal(content1.Length, values[0].Length); + Assert.Equal(content2.Length, values[1].Length); + + Assert.Equal(content1, values[0]); + Assert.Equal(content2, values[1]); + } + } + + + /// + /// A helper function that writes to a stream using the message writer and returns the content that is present in the stream. + /// + /// String content to write. + /// A service provider with the default configurations. + /// A task that resolves to the string present in the output stream. + private static async Task WritePayloadAsync(string content, IServiceProvider serviceProvider) + { + await using Stream outputStream = new MemoryStream(); + + var message = new InMemoryMessage + { + Stream = outputStream, + ServiceProvider = serviceProvider + }; + + var responseMessage = new ODataResponseMessage(message, writing: true, enableMessageStreamDisposal: true, maxMessageSize: -1); + await using ODataMessageWriter writer = new ODataMessageWriter(responseMessage); + + await Task.Yield(); + + await writer.WriteValueAsync(content); + + outputStream.Position = 0; + using var reader = new StreamReader(outputStream); + + await Task.Yield(); + + string written = await reader.ReadToEndAsync(); + await writer.DisposeAsync(); + return written; + } + } +}