diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs b/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs index 4b29c4ca..4548a21c 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.Async.cs @@ -37,9 +37,52 @@ public abstract partial class AbstractWritableArchive } } - // Async property moved from main file private IAsyncEnumerable OldEntriesAsync => - base.EntriesAsync.Where(x => !removedEntries.Contains(x)); + LoadOriginalEntriesAsync().Where(x => !removedEntries.Contains(x)); + + private async IAsyncEnumerable LoadOriginalEntriesAsync( + [EnumeratorCancellation] CancellationToken cancellationToken = default + ) + { + var index = 0; + while (true) + { + while (index < originalEntries.Count) + { + cancellationToken.ThrowIfCancellationRequested(); + yield return originalEntries[index++]; + } + if (originalEntriesLoaded) + { + yield break; + } + + var generation = ++originalEntriesGeneration; + await using var source = LoadEntriesAsync(VolumesAsync) + .GetAsyncEnumerator(cancellationToken); + for (var skipped = 0; skipped < index; skipped++) + { + if (!await source.MoveNextAsync().ConfigureAwait(false)) + { + throw new IncompleteArchiveException("Archive entries changed while loading."); + } + } + while ( + generation == originalEntriesGeneration + && await source.MoveNextAsync().ConfigureAwait(false) + ) + { + cancellationToken.ThrowIfCancellationRequested(); + originalEntries.Add(source.Current); + index++; + yield return source.Current; + } + if (generation == originalEntriesGeneration) + { + originalEntriesLoaded = true; + } + } + } private async ValueTask RebuildModifiedCollectionAsync() { @@ -155,6 +198,7 @@ public abstract partial class AbstractWritableArchive public override async ValueTask DisposeAsync() { await base.DisposeAsync().ConfigureAwait(false); + originalEntries.Cast().ForEach(x => x.Close()); newEntries.Cast().ForEach(x => x.Close()); removedEntries.Cast().ForEach(x => x.Close()); modifiedEntries.Cast().ForEach(x => x.Close()); diff --git a/src/SharpCompress/Archives/AbstractWritableArchive.cs b/src/SharpCompress/Archives/AbstractWritableArchive.cs index bcfc0594..dea9de08 100644 --- a/src/SharpCompress/Archives/AbstractWritableArchive.cs +++ b/src/SharpCompress/Archives/AbstractWritableArchive.cs @@ -41,6 +41,10 @@ public abstract partial class AbstractWritableArchive private readonly List newEntries = new(); private readonly List removedEntries = new(); + private readonly List originalEntries = new(); + private readonly LazyReadOnlyCollection originalEntriesView; + private bool originalEntriesLoaded; + private int originalEntriesGeneration; private readonly List modifiedEntries = new(); private bool hasModifications; @@ -48,10 +52,17 @@ public abstract partial class AbstractWritableArchive private bool asyncRebuildPending; internal AbstractWritableArchive(ArchiveType type) - : base(type) { } + : base(type) + { + originalEntriesLoaded = true; + originalEntriesView = new LazyReadOnlyCollection(LoadOriginalEntries()); + } internal AbstractWritableArchive(ArchiveType type, SourceStream sourceStream) - : base(type, sourceStream) { } + : base(type, sourceStream) + { + originalEntriesView = new LazyReadOnlyCollection(LoadOriginalEntries()); + } public override ICollection Entries { @@ -65,7 +76,7 @@ public abstract partial class AbstractWritableArchive { return modifiedEntries; } - return base.Entries; + return originalEntriesView; } } @@ -84,7 +95,46 @@ public abstract partial class AbstractWritableArchive modifiedEntries.AddRange(OldEntries.Concat(newEntries)); } - private IEnumerable OldEntries => base.Entries.Where(x => !removedEntries.Contains(x)); + private IEnumerable LoadOriginalEntries() + { + var index = 0; + while (true) + { + while (index < originalEntries.Count) + { + yield return originalEntries[index++]; + } + if (originalEntriesLoaded) + { + yield break; + } + + // Both APIs share entry identities. Switching loaders must restart + // parsing: the other loader may have moved the archive's source stream. + var generation = ++originalEntriesGeneration; + using var source = LoadEntries(Volumes).GetEnumerator(); + for (var skipped = 0; skipped < index; skipped++) + { + if (!source.MoveNext()) + { + throw new IncompleteArchiveException("Archive entries changed while loading."); + } + } + while (generation == originalEntriesGeneration && source.MoveNext()) + { + originalEntries.Add(source.Current); + index++; + yield return source.Current; + } + if (generation == originalEntriesGeneration) + { + originalEntriesLoaded = true; + } + } + } + + private IEnumerable OldEntries => + originalEntriesView.Where(x => !removedEntries.Contains(x)); public void RemoveEntry(TEntry entry) { @@ -256,6 +306,7 @@ public abstract partial class AbstractWritableArchive public override void Dispose() { base.Dispose(); + originalEntries.Cast().ForEach(x => x.Close()); newEntries.Cast().ForEach(x => x.Close()); removedEntries.Cast().ForEach(x => x.Close()); modifiedEntries.Cast().ForEach(x => x.Close()); diff --git a/tests/SharpCompress.Test/GZip/GZipArchiveAsyncTests.cs b/tests/SharpCompress.Test/GZip/GZipArchiveAsyncTests.cs index ba5a7782..93651949 100644 --- a/tests/SharpCompress.Test/GZip/GZipArchiveAsyncTests.cs +++ b/tests/SharpCompress.Test/GZip/GZipArchiveAsyncTests.cs @@ -85,7 +85,8 @@ public class GZipArchiveAsyncTests : ArchiveTests #endif await using (var archive = await GZipArchive.OpenAsyncArchive(new AsyncOnlyStream(stream))) { - await Assert.ThrowsAsync(async () => + // Shared cached entries avoid synchronous I/O on the async-only source. + await Assert.ThrowsAsync(async () => await archive.AddEntryAsync("jpg\\test.jpg", File.OpenRead(jpg), closeStream: true) ); await archive.SaveToAsync( diff --git a/tests/SharpCompress.Test/WritableArchiveIdentityTests.cs b/tests/SharpCompress.Test/WritableArchiveIdentityTests.cs new file mode 100644 index 00000000..aad0fd86 --- /dev/null +++ b/tests/SharpCompress.Test/WritableArchiveIdentityTests.cs @@ -0,0 +1,189 @@ +using System; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using SharpCompress.Archives; +using SharpCompress.Archives.Tar; +using SharpCompress.Archives.Zip; +using SharpCompress.Common; +using SharpCompress.Readers; +using SharpCompress.Writers; +using SharpCompress.Writers.Tar; +using SharpCompress.Writers.Zip; +using Xunit; + +namespace SharpCompress.Test; + +public class WritableArchiveIdentityTests +{ + [Theory] + [InlineData(ArchiveType.Zip, false, false)] + [InlineData(ArchiveType.Zip, false, true)] + [InlineData(ArchiveType.Zip, true, false)] + [InlineData(ArchiveType.Zip, true, true)] + [InlineData(ArchiveType.Tar, false, false)] + [InlineData(ArchiveType.Tar, false, true)] + [InlineData(ArchiveType.Tar, true, false)] + [InlineData(ArchiveType.Tar, true, true)] + public async Task Removal_Survives_Switching_Apis_And_Rebuilding( + ArchiveType type, + bool asyncFirst, + bool paused + ) + { + using var input = CreateArchive(type, "remove", "keep"); + using var archive = ArchiveFactory.OpenArchive(input, ReaderOptions.ForExternalStream); + var writable = (IWritableArchive)archive; + var asyncWritable = (IWritableAsyncArchive)archive; + // Leave the first loader partially consumed before switching APIs. + var entry = asyncFirst + ? await asyncWritable.EntriesAsync.FirstAsync(CancellationToken.None) + : archive.Entries.First(); + var otherEntry = asyncFirst + ? archive.Entries.First() + : await asyncWritable.EntriesAsync.FirstAsync(CancellationToken.None); + Assert.Same(entry, otherEntry); + + using var source = new MemoryStream(new byte[] { 42 }); + using (paused ? writable.PauseEntryRebuilding() : null) + { + if (asyncFirst) + { + writable.RemoveEntry(entry); + writable.AddEntry("new", source, false); + } + else + { + await asyncWritable.RemoveEntryAsync(entry); + await asyncWritable.AddEntryAsync( + "new", + source, + false, + cancellationToken: CancellationToken.None + ); + } + } + var syncEntries = archive.Entries.ToList(); + var asyncEntries = await asyncWritable.EntriesAsync.ToListAsync(CancellationToken.None); + Assert.Equal(new[] { "keep", "new" }, syncEntries.Select(e => e.Key)); + Assert.Equal(syncEntries, asyncEntries); + + foreach (var useAsync in new[] { false, true }) + { + using var output = new MemoryStream(); + await Save(archive, output, useAsync); + output.Position = 0; + using var saved = ArchiveFactory.OpenArchive(output, ReaderOptions.ForExternalStream); + Assert.Equal(new[] { "keep", "new" }, saved.Entries.Select(e => e.Key)); + } + Assert.True(input.CanRead); + Assert.True(source.CanRead); + } + + [Theory] + [InlineData(ArchiveType.Zip, false)] + [InlineData(ArchiveType.Zip, true)] + [InlineData(ArchiveType.Tar, false)] + [InlineData(ArchiveType.Tar, true)] + public async Task Shared_Identity_Does_Not_Remove_Other_Entries_With_The_Same_Name( + ArchiveType type, + bool asyncFirst + ) + { + using var input = CreateArchive(type, "same", "same"); + using var archive = ArchiveFactory.OpenArchive(input, ReaderOptions.ForExternalStream); + var asyncArchive = (IWritableAsyncArchive)archive; + if (asyncFirst) + { + var entry = await asyncArchive.EntriesAsync.FirstAsync(CancellationToken.None); + ((IWritableArchive)archive).RemoveEntry(entry); + } + else + { + await asyncArchive.RemoveEntryAsync(archive.Entries.First()); + } + var remaining = Assert.Single(archive.Entries); + Assert.Same( + remaining, + Assert.Single(await asyncArchive.EntriesAsync.ToListAsync(CancellationToken.None)) + ); + using var payload = remaining.OpenEntryStream(); + Assert.Equal(2, payload.ReadByte()); + } + + [Theory] + [InlineData(ArchiveType.Zip)] + [InlineData(ArchiveType.Tar)] + public async Task Partially_Consumed_Loaders_Resume_From_The_Shared_Cache(ArchiveType type) + { + using var input = CreateArchive(type, "first", "second", "third"); + using var archive = ArchiveFactory.OpenArchive(input, ReaderOptions.ForExternalStream); + using var syncEntries = archive.Entries.GetEnumerator(); + await using var asyncEntries = ((IAsyncArchive)archive).EntriesAsync.GetAsyncEnumerator( + CancellationToken.None + ); + Assert.True(syncEntries.MoveNext()); + Assert.True(await asyncEntries.MoveNextAsync()); + Assert.Same(syncEntries.Current, asyncEntries.Current); + Assert.True(await asyncEntries.MoveNextAsync()); + var second = asyncEntries.Current; + Assert.True(syncEntries.MoveNext()); + Assert.Same(second, syncEntries.Current); + Assert.True(syncEntries.MoveNext()); + Assert.True(await asyncEntries.MoveNextAsync()); + Assert.Same(syncEntries.Current, asyncEntries.Current); + Assert.False(syncEntries.MoveNext()); + Assert.False(await asyncEntries.MoveNextAsync()); + } + + private static MemoryStream CreateArchive(ArchiveType type, params string[] names) + { + var input = new MemoryStream(); + using ( + var writer = WriterFactory.OpenWriter( + input, + type, + new WriterOptions(CompressionType.None) { LeaveStreamOpen = true } + ) + ) + { + for (var i = 0; i < names.Length; i++) + { + using var source = new MemoryStream(new[] { (byte)(i + 1) }); + writer.Write(names[i], source, DateTime.UtcNow); + } + } + input.Position = 0; + return input; + } + + private static async Task Save(IArchive archive, Stream output, bool useAsync) + { + if (archive is ZipArchive zip) + { + var options = new ZipWriterOptions(CompressionType.None) { LeaveStreamOpen = true }; + if (useAsync) + { + await zip.SaveToAsync(output, options, CancellationToken.None); + } + else + { + zip.SaveTo(output, options); + } + } + else + { + var tar = (TarArchive)archive; + var options = new TarWriterOptions(CompressionType.None) { LeaveStreamOpen = true }; + if (useAsync) + { + await tar.SaveToAsync(output, options, CancellationToken.None); + } + else + { + tar.SaveTo(output, options); + } + } + } +}