Await File Download From Rest API

[madaxe2020]

I'm downloading files from a Jira Task, and while debugging my code exists before the file is downloaded as a result the thread is exiting and the file is not downloading. i added the do loop and this gives enough time for the file to download, but I don't like it.

how can i stop the code form completing before all awaited threads have finished?

thanks

Madaxe

public bool DownloadAttachment(string attachmentId, string folderPath)
{
    Attachment? attachment = (from x in _attachments where x.id == attachmentId select x).FirstOrDefault()??null;
    if (attachment != null)
    {
        string filePath = string.Concat(folderPath, Path.DirectorySeparatorChar, attachment.filename);
        var bob = EnvironmentMangerHTTPClient.DownloadTaskAttachmentAsync(attachmentId, filePath);
        var startTime = DateTime.UtcNow;
        //Allow upto 1 minute before returning false
        do
        {
            if(DateTime.UtcNow - startTime == TimeSpan.FromMinutes(1))
            {
                return false;
            }
        } while (File.Exists(filePath)==false);
        return true;
    }
    else
    {
        return false;
    }

}

public static async Task<bool> DownloadTaskAttachmentAsync(string attachmentId, string filePath)
{
    if (!String.IsNullOrEmpty(apiKey) && !String.IsNullOrEmpty(username))
    {
        var returnstatus = await GetAttachment(String.Concat("/rest/api/3/attachment/content/", attachmentId), filePath);
        return true;
    }
    throw new Exception("Environment Variables jiraapikey and jirausername were invalid.");
}

private static async Task<bool> GetAttachment(string url, string filePath)
{
    try
    {
        using (var client = new HttpClient())
        {
            client.BaseAddress = new Uri(apiBaseUri);
            client.DefaultRequestHeaders.Add("Authorization", $"Basic {credentials}");
            var dataStream = await client.GetStreamAsync(url);
            string fileToWriteTo = string.Concat(filePath);
            using Stream streamToWriteTo = File.Open(fileToWriteTo, FileMode.Create);
            {
                await dataStream.CopyToAsync(streamToWriteTo);
            }
            return true;
        }
    }
    catch(Exception ex)
    {
        Console.Write(ex.ToString());
        return false;
    }
}

 

[JohnH]

"async all the way" - in DownloadAttachment you resort to polling in the worst possible way. DownloadTaskAttachmentAsync should be awaited.
General rule is only create one HttpClient and share it for lifetime of application.
If you need timeout set this on HttpClient, or use a CancellationTokenSource with CancelAfter and pass the token to GetStreamAsync.

 

[cjard]

You can install the Flurl.Http nuget package then swap out all that code for something like:

foreach(var a in _attachments){
  await $"http://host.goes.here/rest/api/3/attachment/content/{a.id}"
    .WithBasicAuth(Environment.GetEnvironmentVariable("jirausername"), Environment.GetEnvironmentVariable("jirausername"))
    .DownloadFileAsync(folder_path_here, file_path_here);
}

ps; don't use string.Concat on strings, and don't use tight loops on anything ever - especially not with `async`

 

[Edwin Klesman]

Just replacing the (never do this!) loop delaying mechanism you created, you could do something like:

public bool DownloadAttachment(string attachmentId, string folderPath)
{
    Attachment? attachment = (from x in _attachments where x.id == attachmentId select x).FirstOrDefault()??null;
    if (attachment != null)
    {
        string filePath = string.Concat(folderPath, Path.DirectorySeparatorChar, attachment.filename);
        var downloadTask = EnvironmentMangerHTTPClient.DownloadTaskAttachmentAsync(attachmentId, filePath);
        downloadTask.Wait(); // Wait for the download task to complete before continuing
        return File.Exists(filePath);
    }
    else
    {
        return false;
    }
}

But, this simply blocks the current thread in a "syntax correct way" in C#.
As @JohnH indicates, going async throughout your code is way neater.
Without using packages that make the syntaxt cleaner and easier (thanks @cjard, for that tip), your code would look like this using async/await:

public async Task<bool> DownloadAttachmentAsync(string attachmentId, string folderPath)
{
    Attachment? attachment = (from x in _attachments where x.id == attachmentId select x).FirstOrDefault() ?? null;
    if (attachment != null)
    {
        string filePath = string.Concat(folderPath, Path.DirectorySeparatorChar, attachment.filename);
        bool downloadResult = await EnvironmentMangerHTTPClient.DownloadTaskAttachmentAsync(attachmentId, filePath);
        return downloadResult && File.Exists(filePath);
    }
    else
    {
        return false;
    }
}

Note that since the DownloadAttachment method now returns a Task<bool>, you need to use await when calling it from another method that wants to wait for the download to complete:

var succesfulDownload = await DownloadAttachmentAsync(attachmentId, filePath);

 

[Edwin Klesman]

(And strive to call it DownloadAttachmentAsync)

Thanks. I've updated my answer, as it's good practice to postfix a function with "Async" so it's easy to spot it's an async function.