dotnet / dotnet/runtime

Reduce code duplication across sync/async in ZIP and TAR

Open
#127,376 2 comments 0 reactions 0 assignees View on GitHub
area-System.IO.Compression
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

Current TarReader/Writer and ZipArchive contain lots of logic, that is duplicated across the sync and async API implementations. See e.g.

https://github.com/dotnet/runtime/blob/6f2ad2d6d76430b0f7c5bddabb64d9957092453a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarReader.cs#L199-L262

Similarly, when introducing new tests, the developers often resort to duplicating the same test across sync and async variants, see e.g. `TarWriter_WriteEntryAsync_File_Tests` and `TarWriter_WriteEntry_File_Tests`. This risks increased regressions where bugs may be fixed in one code path but not the other (and not caught because only one variant of test was added). Deduplicating the logic would decrease developer toil and reduce the risk of regressions.

Since the code generallly boils down to calling a sync/async variant of a Read or Write method on a Stream, we can use the same technique of deduplicating the implementation as we did in SslStream and SmtpClient, see e.g. https://github.com/dotnet/runtime/pull/115366.

IReadWriteAdapter

https://github.com/dotnet/runtime/blob/6f2ad2d6d76430b0f7c5bddabb64d9957092453a/src/libraries/Common/src/System/Net/ReadWriteAdapter.cs#L10-L75

Usage example:

https://github.com/dotnet/runtime/blob/6f2ad2d6d76430b0f7c5bddabb64d9957092453a/src/libraries/System.Net.Mail/src/System/Net/Mail/SmtpReplyReaderFactory.cs#L251-L281

https://github.com/dotnet/runtime/blob/6f2ad2d6d76430b0f7c5bddabb64d9957092453a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs#L806-L813

https://github.com/dotnet/runtime/blob/6f2ad2d6d76430b0f7c5bddabb64d9957092453a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs#L890-L895

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.