Skip to content
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

Allow async ImapFetchStreamCallback for ImapFolder.GetStreams methods #958

Closed
lkstz opened this issue Nov 29, 2019 · 2 comments
Closed

Allow async ImapFetchStreamCallback for ImapFolder.GetStreams methods #958

lkstz opened this issue Nov 29, 2019 · 2 comments
Labels
enhancement New feature or request

Comments

@lkstz
Copy link

lkstz commented Nov 29, 2019

Is your feature request related to a problem? Please describe.
When downloading multiple messages using one of the ImapFolder.GetStreams overloads, it is currently not possible to use an async method as ImapFetchStreamCallback. Many .NET APIs are async-first if not async-only nowadays, so one has to build complex workarounds to allow async processing of the messages.

Describe the solution you'd like
I'd like to have an async version of ImapFetchStreamCallback, e.g. ImapFetchStreamAsyncCallback:

public delegate Task ImapFetchStreamAsyncCallback (ImapFolder folder, int index, UniqueId uid, Stream stream);

Each ImapFolder.GetStreams method would get an overload accepting an ImapFetchStreamAsyncCallback.


If this feature is not too complicated, I'd be open to implement it and make a PR.

@lkstz lkstz changed the title Allow async ImapFetchStreamCallback for GetStreams methods Allow async ImapFetchStreamCallback for ImapFolder.GetStreams methods Nov 29, 2019
@jstedfast
Copy link
Owner

Hmmm, yes, this should have been an async callback. I must have missed this when I rewrote MailKit to be async from the ground up in the 2.0 rewrite.

I think the solution for now is probably to fix the GetStreamsAsync() methods to take an ImapFetchStreamAsyncCallback delegate instead of duplicating the API (which I really hate doing).

I'll leave the GetStreams() APIs alone, so they will continue to use the old callback delegate (which probably makes sense anyway since they probably don't care about being async).

In a future version, I'll change the ImapFetchStreamCallback delegate signature to also take a CancellationToken argument like I'm adding to the ImapFetchStreamAsyncCallback delegate signature.

jstedfast added a commit that referenced this issue Nov 29, 2019
Fixes issue #958

Note: Developers using these APIs will need to update their code as
this change breaks API/ABI.
@jstedfast jstedfast added the enhancement New feature or request label Nov 29, 2019
@lkstz
Copy link
Author

lkstz commented Nov 29, 2019

👍 Thank you for the fast reaction!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants