BaseJob: prepare() -> initiate() + refactoring around it

* BaseJob::initiate() now calls ConnectionData::submit()
  without relying on Connection to do that
* ConnectionData::submit() is now the only site where a job enters
  Pending state
* No more shortcuts to BaseJob::sendRequest(), even retries are sent
  through the ConnectionData submission queue
* Additional validation in BaseJob::initiate() that the request data
  device is actually open (because QtNetwork API officially
  requires that, even if you can get away passing a closed QBuffer
  to it)

2 files changed, 20 insertions, 10 deletions

diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp
index 7f558685..4653e58a 100644
--- a/lib/jobs/basejob.cpp
+++ b/lib/jobs/basejob.cpp
@@ -138,8 +138,7 @@ BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint,
     setObjectName(name);
     connect(&d->timer, &QTimer::timeout, this, &BaseJob::timeout);
     connect(&d->retryTimer, &QTimer::timeout, this, [this] {
-        setStatus(Pending);
-        sendRequest();
+        d->connection->submit(this);
     });
 }
 
@@ -259,14 +258,27 @@ void BaseJob::onSentRequest(QNetworkReply*) {}
 
 void BaseJob::beforeAbandon(QNetworkReply*) {}
 
-void BaseJob::prepare(ConnectionData* connData, bool inBackground)
+void BaseJob::initiate(ConnectionData* connData, bool inBackground)
 {
+    Q_ASSERT(connData != nullptr);
+
     d->inBackground = inBackground;
     d->connection = connData;
     doPrepare();
-    if (status().code != Unprepared && status().code != Pending)
+
+    if ((d->verb == HttpVerb::Post || d->verb == HttpVerb::Put)
+        && !d->requestData.source()->isReadable()) {
+        setStatus(FileError, "Request data not ready");
+    }
+    Q_ASSERT(status().code != Pending); // doPrepare() must NOT set this
+    if (status().code == Unprepared) {
+        d->connection->submit(this);
+    } else {
+        qDebug(d->logCat).noquote()
+            << "Request failed preparation and won't be sent:"
+            << d->dumpRequest();
         QTimer::singleShot(0, this, &BaseJob::finishJob);
-    setStatus(Pending);
+    }
 }
 
 void BaseJob::sendRequest()
@@ -292,7 +304,7 @@ void BaseJob::sendRequest()
         onSentRequest(d->reply.data());
         emit sentRequest();
     } else
-        qCWarning(d->logCat).noquote()
+        qCCritical(d->logCat).noquote()
             << "Request could not start:" << d->dumpRequest();
 }
 
@@ -494,7 +506,6 @@ void BaseJob::finishJob()
     stop();
     if (error() == TooManyRequests) {
         emit rateLimited();
-        setStatus(Pending);
         d->connection->submit(this);
         return;
     }
@@ -505,8 +516,7 @@ void BaseJob::finishJob()
         d->connection->setNeedsToken(objectName());
         qCWarning(d->logCat) << this << "re-running with authentication";
         emit retryScheduled(d->retriesTaken, 0);
-        setStatus(Pending);
-        sendRequest();
+        d->connection->submit(this);
     }
     if ((error() == NetworkError || error() == Timeout)
         && d->retriesTaken < d->maxRetries) {
diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h
index e708ba8d..c8046e9e 100644
--- a/lib/jobs/basejob.h
+++ b/lib/jobs/basejob.h
@@ -203,7 +203,7 @@ public:
     }
 
 public slots:
-    void prepare(ConnectionData* connData, bool inBackground);
+    void initiate(ConnectionData* connData, bool inBackground);
 
     /**
      * Abandons the result of this job, arrived or unarrived.