File Upload with Progress Window Thread

I have been looking everywhere for a clear example on how to upload a file with a Progress Bar. After some blog scraping, I managed to put together a simple class which demonstrates how to do this.

The code compiles, but I haven’t tested it, but it is my guesstimate that it’ll work right out of the box.

class API_Set_File_Upload : public ThreadWithProgressWindow {
public:
    API_Set_File_Upload(File file_to_upload, String host_name)
    : ThreadWithProgressWindow("Uploading file " + file_to_upload.getFileName(), true, true, 1000, "Cancel")
    , file_to_upload_(file_to_upload)
	, host_name_(host_name)
    {
	}
	
    virtual void run() override {
	    
	    if (!file_to_upload_.existsAsFile()){
	        response_str = "Upload file does not exist.";
	        return;
	    }
	    
	    // open a URL connection to you Rest server.
		String url_str(host_name_ + "/files");
	    URL url(url_str);
		
	    url = url.withFileToUpload("file", file_to_upload_, "application/octet-stream");
	    URL::OpenStreamProgressCallback* callback = &API_Set_File_Upload::ProgressCallback;
	    InputStream* input = url.createInputStream(true, callback, this);
	
	    String result_post = input->readEntireStreamAsString();
	    DBG("result post: " + result_post);
	    response_str += String("one upload done");
        
    }
    
    static bool ProgressCallback(void *context, int bytesSent, int totalBytes){
        double progress_value = bytesSent / ((double)totalBytes);
        DBG("progress: " + String(progress_value));
        if (static_cast<API_Set_File_Upload*>(context)->currentThreadShouldExit())
            return false;
        static_cast<API_Set_File_Upload*>(context)->setProgress(progress_value);
        return true;
    }
    String getResponseString() { ScopedLock l(responseLock); return response; }


    
protected:
    
    File file_to_upload_;
	String host_name_;
	String response_str = "ok";
    
};

Enjoy !

1 Like

I’m not sure what your intention is with the CriticalSection, but two things, first the line in run which attempts to take a lock,
ScopedLock Lock(CriticalSection);
is creating a local CriticalSection object and taking a lock on that, not on your member variable
CriticalSection critical_section

Second, you don’t use the CriticalSection anywhere else in the code, so as I said, I am not sure what your intention is in using it.

Hi,
You’re right it’s confusing so I edited the post.
Thank you !
J

1 Like

Jacques, your thread safety here comes from setting the member variables during construction and then only accessing them from the background thread. So I think you are almost fine without that lock, except for “response_str” which will need a lock if you ever want to read it while the thread is running.

Maybe make it private member so you can’t access it any other way then two member functions that must always be used when accessing the string:

String getResponseString() { ScopedLock l(responseLock); return response; }

void setResponseString(String newString) { ScopedLock l(responseLock);response = newString; }

And of course a new member variable:

CriticalSection responseLock;

1 Like

Jim you are right, however I make the user wait for the file to upload, like such:

            File file(file_path);
            API_Set_File_Upload file_upload(file, host_name);
            if (file_upload.runThread())
            {
                DBG("Finished uploading file " + file.getFileName());
            }
            else
            {
                DBG("File upload was canceled by user");
            }

Since these things can take quite a lot of time at times, I prefer to give the user a chance to cancel.

This means the response_str will be safe if not reached for before “runThread” finishes, and so can be used mostly for error processing.

Would this be right, Jim ?

You might be right, but I think for the sake for 5 extra lines of code that’ll prevent you ever shooting yourself in the foot it’ll be worth adding the CriticalSection :wink:

You dont’ know what changes you’ll make in the future …

1 Like

I made some changes then.
I left out the “setResponse()” because in this case it’s going to be something you fetch, but never set.

Cheers !!!