-
Notifications
You must be signed in to change notification settings - Fork 479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new event handler to allow tracking of progress of extraction progress for individual entry #226
Add new event handler to allow tracking of progress of extraction progress for individual entry #226
Changes from 3 commits
467fc2d
b8ef1ec
683d271
e05f984
0990b06
2aa123c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ public abstract class AbstractReader<TEntry, TVolume> : IReader, IReaderExtracti | |
|
||
public event EventHandler<ReaderExtractionEventArgs<IEntry>> EntryExtractionBegin; | ||
public event EventHandler<ReaderExtractionEventArgs<IEntry>> EntryExtractionEnd; | ||
public event EventHandler<ReaderExtractionEventArgs<IEntry>> EntryExtractionProgress; | ||
|
||
public event EventHandler<CompressedBytesReadEventArgs> CompressedBytesRead; | ||
public event EventHandler<FilePartExtractionBeginEventArgs> FilePartExtractionBegin; | ||
|
@@ -181,16 +182,16 @@ public void WriteEntryTo(Stream writableStream) | |
|
||
var streamListener = this as IReaderExtractionListener; | ||
streamListener.FireEntryExtractionBegin(Entry); | ||
Write(writableStream); | ||
Write(writableStream, streamListener); | ||
streamListener.FireEntryExtractionEnd(Entry); | ||
wroteCurrentEntry = true; | ||
} | ||
|
||
internal void Write(Stream writeStream) | ||
internal void Write(Stream writeStream, IReaderExtractionListener streamListener) | ||
{ | ||
using (Stream s = OpenEntryStream()) | ||
{ | ||
s.TransferTo(writeStream); | ||
s.TransferTo(writeStream, (sizeTransferred, iterations) => streamListener.FireEntryExtractionProgress(Entry, sizeTransferred, iterations)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this is okay. I wouldn't have a problem just duplicating the transfer code to have listening and not listening. Kind of doesn't make sense to have a lambda just to call the listener when the listener can just be passed in. |
||
} | ||
} | ||
|
||
|
@@ -255,6 +256,14 @@ void IReaderExtractionListener.FireEntryExtractionBegin(Entry entry) | |
} | ||
} | ||
|
||
void IReaderExtractionListener.FireEntryExtractionProgress(Entry entry, long bytesTransferred, int iterations) | ||
{ | ||
if (EntryExtractionProgress != null) | ||
{ | ||
EntryExtractionProgress(this, new ReaderExtractionEventArgs<IEntry>(entry, new ReaderProgress(entry, bytesTransferred, iterations))); | ||
} | ||
} | ||
|
||
void IReaderExtractionListener.FireEntryExtractionEnd(Entry entry) | ||
{ | ||
if (EntryExtractionEnd != null) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ public interface IReader : IDisposable | |
{ | ||
event EventHandler<ReaderExtractionEventArgs<IEntry>> EntryExtractionBegin; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would dump the Begin and End and only have Progress. They can check iterations or something to see if it's started. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I sort of agree, but this would break current implementations using those event handlers. Would this be ok? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I break things all the time :) I keep this library "beta" just because I haven't given things enough thought nor have the time to worry about API breaking on updates. Users should expect to at least recompile to find API breakages. |
||
event EventHandler<ReaderExtractionEventArgs<IEntry>> EntryExtractionEnd; | ||
event EventHandler<ReaderExtractionEventArgs<IEntry>> EntryExtractionProgress; | ||
|
||
event EventHandler<CompressedBytesReadEventArgs> CompressedBytesRead; | ||
event EventHandler<FilePartExtractionBeginEventArgs> FilePartExtractionBegin; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
| ||
|
||
using System; | ||
using SharpCompress.Common; | ||
|
||
namespace SharpCompress.Readers | ||
{ | ||
public class ReaderProgress | ||
{ | ||
private readonly IEntry _entry; | ||
public long BytesTransferred { get; private set; } | ||
public int Iterations { get; private set; } | ||
|
||
public int PercentageRead => (int)Math.Round(PercentageReadExact); | ||
public double PercentageReadExact => (float)BytesTransferred / _entry.Size * 100; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the way this was done |
||
|
||
public ReaderProgress(IEntry entry, long bytesTransferred, int iterations) | ||
{ | ||
_entry = entry; | ||
BytesTransferred = bytesTransferred; | ||
Iterations = iterations; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -229,15 +229,18 @@ public static DateTime DosDateToDateTime(Int32 iTime) | |
return DosDateToDateTime((UInt32)iTime); | ||
} | ||
|
||
public static long TransferTo(this Stream source, Stream destination) | ||
public static long TransferTo(this Stream source, Stream destination, Action<long, int> action = null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, I would just pass IStreamListener as an optional value like this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I stated, my thought process was to not introduce a new dependency into utility, but if you are fine with this I will change to that behavior. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This Utility won't be reused and it's just for the library. I'm fine with internal dependencies. |
||
{ | ||
byte[] array = new byte[81920]; | ||
int count; | ||
var iterations = 0; | ||
long total = 0; | ||
while ((count = source.Read(array, 0, array.Length)) != 0) | ||
{ | ||
total += count; | ||
destination.Write(array, 0, count); | ||
iterations++; | ||
action?.Invoke(total, iterations); | ||
} | ||
return total; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this strongly typed. I don't see a need for this as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea here was to generalize what could be passed, but am I correct in assuming that you want ReaderProgress here instead of object[] ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. We'll worry about other options later. No need to over genericize at the moment I think.