ShareDialog: Improve error reporting for share API fails.

This would have made debugging #3204 and #3136 easier.

src/gui/sharedialog.cpp

@@ -144,6 +144,15 @@ void ShareDialog::done( int r ) {
     QDialog::done(r);
 }
 
+static int getJsonReturnCode(const QVariantMap &json, QString &message)
+{
+    //TODO proper checking
+    int code = json.value("ocs").toMap().value("meta").toMap().value("statuscode").toInt();
+    message = json.value("ocs").toMap().value("meta").toMap().value("message").toString();
+
+    return code;
+}
+
 void ShareDialog::setExpireDate(const QDate &date)
 {
     if( _public_share_id == 0 ) {
@@ -162,16 +171,14 @@ void ShareDialog::setExpireDate(const QDate &date)
 
     OcsShareJob *job = new OcsShareJob("PUT", url, _account, this);
     job->setPostParams(postParams);
-    connect(job, SIGNAL(jobFinished(QString)), this, SLOT(slotExpireSet(QString)));
+    connect(job, SIGNAL(jobFinished(QVariantMap)), this, SLOT(slotExpireSet(QVariantMap)));
     job->start();
 }
 
-void ShareDialog::slotExpireSet(const QString &reply)
+void ShareDialog::slotExpireSet(const QVariantMap &reply)
 {
     QString message;
-    int code = checkJsonReturnCode(reply, message);
+    int code = getJsonReturnCode(reply, message);
-
-    qDebug() << Q_FUNC_INFO << "Status code: " << code;
     if (code != 100) {
         displayError(code);
     } 
@@ -234,18 +241,15 @@ void ShareDialog::setPassword(const QString &password)
     }
     OcsShareJob *job = new OcsShareJob(verb, url, _account, this);
     job->setPostParams(requestParams);
-    connect(job, SIGNAL(jobFinished(QString)), this, SLOT(slotPasswordSet(QString)));
+    connect(job, SIGNAL(jobFinished(QVariantMap)), this, SLOT(slotPasswordSet(QVariantMap)));
     job->start();
     _passwordJobRunning = true;
 }
 
-void ShareDialog::slotPasswordSet(const QString &reply)
+void ShareDialog::slotPasswordSet(const QVariantMap &reply)
 {
     QString message;
-    int code = checkJsonReturnCode(reply, message);
+    int code = getJsonReturnCode(reply, message);
-
-    qDebug() << Q_FUNC_INFO << "Status code: " << code;
-
     if (code != 100) {
         displayError(code);
     }
@@ -267,23 +271,20 @@ void ShareDialog::getShares()
     params.append(qMakePair(QString::fromLatin1("path"), _sharePath));
     url.setQueryItems(params);
     OcsShareJob *job = new OcsShareJob("GET", url, _account, this);
-    connect(job, SIGNAL(jobFinished(QString)), this, SLOT(slotSharesFetched(QString)));
+    job->addPassStatusCode(404); // don't report error if share doesn't exist yet
+    connect(job, SIGNAL(jobFinished(QVariantMap)), this, SLOT(slotSharesFetched(QVariantMap)));
     job->start();
 }
 
-void ShareDialog::slotSharesFetched(const QString &reply)
+void ShareDialog::slotSharesFetched(const QVariantMap &reply)
 {
     QString message;
-    int code = checkJsonReturnCode(reply, message);
+    int code = getJsonReturnCode(reply, message);
-
-    qDebug() << Q_FUNC_INFO << "Status code: " << code;
     if (code != 100 && code != 404) {
         displayError(code);
     }
 
-    bool success = false;
+    ShareDialog::_shares = reply.value("ocs").toMap().value("data").toList();
-    QVariantMap json = QtJson::parse(reply, success).toMap();
-    ShareDialog::_shares = json.value("ocs").toMap().value("data").toList();
     const QString versionString = AccountManager::instance()->account()->serverVersion();
 
     Q_FOREACH(auto share, ShareDialog::_shares) {
@@ -384,12 +385,10 @@ void ShareDialog::setShareLink( const QString& url )
 
 }
 
-void ShareDialog::slotDeleteShareFetched(const QString &reply)
+void ShareDialog::slotDeleteShareFetched(const QVariantMap &reply)
 {
     QString message;
-    int code = checkJsonReturnCode(reply, message);
+    int code = getJsonReturnCode(reply, message);
-
-    qDebug() << Q_FUNC_INFO << "Status code: " << code;
     if (code != 100) {
         displayError(code);
     }
@@ -423,21 +422,22 @@ void ShareDialog::slotCheckBoxShareLinkClicked()
         postParams.append(qMakePair(QString::fromLatin1("shareType"), QString::number(SHARETYPE_PUBLIC)));
         OcsShareJob *job = new OcsShareJob("POST", url, _account, this);
         job->setPostParams(postParams);
-        connect(job, SIGNAL(jobFinished(QString)), this, SLOT(slotCreateShareFetched(QString)));
+        job->addPassStatusCode(403); // "password required" is not an error
+        connect(job, SIGNAL(jobFinished(QVariantMap)), this, SLOT(slotCreateShareFetched(QVariantMap)));
         job->start();
     } else {
         _pi_link->startAnimation();
         QUrl url = Account::concatUrlPath(_account->url(), QString("ocs/v1.php/apps/files_sharing/api/v1/shares/%1").arg(_public_share_id));
         OcsShareJob *job = new OcsShareJob("DELETE", url, _account, this);
-        connect(job, SIGNAL(jobFinished(QString)), this, SLOT(slotDeleteShareFetched(QString)));
+        connect(job, SIGNAL(jobFinished(QVariantMap)), this, SLOT(slotDeleteShareFetched(QVariantMap)));
         job->start();
     }
 }
 
-void ShareDialog::slotCreateShareFetched(const QString &reply)
+void ShareDialog::slotCreateShareFetched(const QVariantMap &reply)
 {
     QString message;
-    int code = checkJsonReturnCode(reply, message);
+    int code = getJsonReturnCode(reply, message);
     _pi_link->stopAnimation();
 
     if (code == 403) {
@@ -455,9 +455,7 @@ void ShareDialog::slotCreateShareFetched(const QString &reply)
         return;
     }
 
-    bool success;
+    _public_share_id = reply.value("ocs").toMap().values("data")[0].toMap().value("id").toULongLong();
-    QVariantMap json = QtJson::parse(reply, success).toMap();
-    _public_share_id = json.value("ocs").toMap().values("data")[0].toMap().value("id").toULongLong();
     getShares();
 }
 
@@ -500,22 +498,6 @@ void ShareDialog::slotPushButtonCopyLinkPressed()
     clipboard->setText(_shareUrl);
 }
 
-int ShareDialog::checkJsonReturnCode(const QString &reply, QString &message)
-{
-    bool success;
-    QVariantMap json = QtJson::parse(reply, success).toMap();
-
-    if (!success) {
-        qDebug() << Q_FUNC_INFO << "Failed to parse reply";
-    }
-
-    //TODO proper checking
-    int code = json.value("ocs").toMap().value("meta").toMap().value("statuscode").toInt();
-    message = json.value("ocs").toMap().value("meta").toMap().value("message").toString();
-
-    return code;
-}
-
 void ShareDialog::setShareCheckBoxTitle(bool haveShares)
 {
     const QString noSharesTitle(tr("&Share link"));
@@ -672,6 +654,7 @@ OcsShareJob::OcsShareJob(const QByteArray &verb, const QUrl &url, AccountPtr acc
   _verb(verb),
   _url(url)
 {
+    _passStatusCodes.append(100);
     setIgnoreCredentialFailure(true);
 }
 
@@ -680,6 +663,11 @@ void OcsShareJob::setPostParams(const QList<QPair<QString, QString> >& postParam
     _postParams = postParams;
 }
 
+void OcsShareJob::addPassStatusCode(int code)
+{
+    _passStatusCodes.append(code);
+}
+
 void OcsShareJob::start()
 {
     QNetworkRequest req;
@@ -711,7 +699,23 @@ void OcsShareJob::start()
 
 bool OcsShareJob::finished()
 {
-    emit jobFinished(reply()->readAll());
+    const QString replyData = reply()->readAll();
+
+    bool success;
+    QVariantMap json = QtJson::parse(replyData, success).toMap();
+    if (!success) {
+        qDebug() << "Could not parse reply to" << _verb << _url << _postParams
+                 << ":" << replyData;
+    }
+
+    QString message;
+    const int statusCode = getJsonReturnCode(json, message);
+    if (!_passStatusCodes.contains(statusCode)) {
+        qDebug() << "Reply to" << _verb << _url << _postParams
+                 << "has unexpected status code:" << statusCode << replyData;
+    }
+
+    emit jobFinished(json);
     return true;
 }
 

src/gui/sharedialog.h

@@ -29,17 +29,19 @@ public:
     explicit OcsShareJob(const QByteArray& verb, const QUrl& url, AccountPtr account, QObject* parent = 0);
 
     void setPostParams(const QList<QPair<QString, QString> >& postParams);
+    void addPassStatusCode(int code);
 
 public slots:
     void start() Q_DECL_OVERRIDE;
 signals:
-    void jobFinished(QString reply);
+    void jobFinished(QVariantMap reply);
 private slots:
     virtual bool finished() Q_DECL_OVERRIDE;
 private:
     QByteArray _verb;
     QUrl _url;
     QList<QPair<QString, QString> > _postParams;
+    QVector<int> _passStatusCodes;
 };
 
 
@@ -62,11 +64,11 @@ public:
     void getShares();
 
 private slots:
-    void slotSharesFetched(const QString &reply);
+    void slotSharesFetched(const QVariantMap &reply);
-    void slotCreateShareFetched(const QString &reply);
+    void slotCreateShareFetched(const QVariantMap &reply);
-    void slotDeleteShareFetched(const QString &reply);
+    void slotDeleteShareFetched(const QVariantMap &reply);
-    void slotPasswordSet(const QString &reply);
+    void slotPasswordSet(const QVariantMap &reply);
-    void slotExpireSet(const QString &reply);
+    void slotExpireSet(const QVariantMap &reply);
     void slotCalendarClicked(const QDate &date);
     void slotCheckBoxShareLinkClicked();
     void slotCheckBoxPasswordClicked();
@@ -101,7 +103,6 @@ private:
     qulonglong _public_share_id;
     void setPassword(const QString &password);
     void setExpireDate(const QDate &date);
-    int checkJsonReturnCode(const QString &reply, QString &message);
 
     QProgressIndicator *_pi_link;
     QProgressIndicator *_pi_password;