Merge pull request #247 from nextcloud/adds-etag-navigation-apps

Checks for 304 response in the navigation apps request

8 files changed, 135 insertions, 51 deletions

diff --git a/src/gui/accountstate.cpp b/src/gui/accountstate.cpp
index 226f42c4a..f0698c248 100644
--- a/src/gui/accountstate.cpp
+++ b/src/gui/accountstate.cpp
@@ -187,6 +187,16 @@ void AccountState::setNotificationsEtagResponseHeader(const QByteArray &value)
     _notificationsEtagResponseHeader = value;
 }
 
+QByteArray AccountState::navigationAppsEtagResponseHeader() const
+{
+    return _navigationAppsEtagResponseHeader;
+}
+
+void AccountState::setNavigationAppsEtagResponseHeader(const QByteArray &value)
+{
+    _navigationAppsEtagResponseHeader = value;
+}
+
 void AccountState::checkConnectivity()
 {
     if (isSignedOut() || _waitingForNewCredentials) {
diff --git a/src/gui/accountstate.h b/src/gui/accountstate.h
index 0aca22ce2..96b840c64 100644
--- a/src/gui/accountstate.h
+++ b/src/gui/accountstate.h
@@ -141,6 +141,16 @@ public:
     */
     void setNotificationsEtagResponseHeader(const QByteArray &value);
 
+    /** Saves the ETag Response header from the last Navigation Apps api
+     * request with statusCode 200.
+    */
+    QByteArray navigationAppsEtagResponseHeader() const;
+
+    /** Returns the ETag Response header from the last Navigation Apps api
+     * request with statusCode 200.
+    */
+    void setNavigationAppsEtagResponseHeader(const QByteArray &value);
+
 public slots:
     /// Triggers a ping to the server to update state and
     /// connection status and errors.
@@ -168,6 +178,7 @@ private:
     QElapsedTimer _timeSinceLastETagCheck;
     QPointer<ConnectionValidator> _connectionValidator;
     QByteArray _notificationsEtagResponseHeader;
+    QByteArray _navigationAppsEtagResponseHeader;
 
     /**
      * Starts counting when the server starts being back up after 503 or
diff --git a/src/gui/ocsjob.cpp b/src/gui/ocsjob.cpp
index e06d7a43a..11e2deffe 100644
--- a/src/gui/ocsjob.cpp
+++ b/src/gui/ocsjob.cpp
@@ -29,6 +29,7 @@ OcsJob::OcsJob(AccountPtr account)
 {
     _passStatusCodes.append(OCS_SUCCESS_STATUS_CODE);
     _passStatusCodes.append(OCS_SUCCESS_STATUS_CODE_V2);
+    _passStatusCodes.append(OCS_NOT_MODIFIED_STATUS_CODE_V2);
     setIgnoreCredentialFailure(true);
 }
 
@@ -52,6 +53,11 @@ void OcsJob::appendPath(const QString &id)
     setPath(path() + QLatin1Char('/') + id);
 }
 
+void OcsJob::addRawHeader(const QByteArray &headerName, const QByteArray &value)
+{
+    _request.setRawHeader(headerName, value);
+}
+
 static QUrlQuery percentEncodeQueryItems(
     const QList<QPair<QString, QString>> &items)
 {
@@ -68,9 +74,8 @@ static QUrlQuery percentEncodeQueryItems(
 
 void OcsJob::start()
 {
-    QNetworkRequest req;
-    req.setRawHeader("Ocs-APIREQUEST", "true");
-    req.setRawHeader("Content-Type", "application/x-www-form-urlencoded");
+    addRawHeader("Ocs-APIREQUEST", "true");
+    addRawHeader("Content-Type", "application/x-www-form-urlencoded");
 
     QBuffer *buffer = new QBuffer;
 
@@ -92,7 +97,7 @@ void OcsJob::start()
     }
     queryItems.addQueryItem(QLatin1String("format"), QLatin1String("json"));
     QUrl url = Utility::concatUrlPath(account()->url(), path(), queryItems);
-    sendRequest(_verb, url, req, buffer);
+    sendRequest(_verb, url, _request, buffer);
     AbstractNetworkJob::start();
 }
 
@@ -101,18 +106,24 @@ bool OcsJob::finished()
     const QByteArray replyData = reply()->readAll();
 
     QJsonParseError error;
+    QString message;
+    int statusCode = 0;
     auto json = QJsonDocument::fromJson(replyData, &error);
+
+    // when it is null we might have a 304 so get status code from reply() and gives a warning...
     if (error.error != QJsonParseError::NoError) {
+        statusCode = reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
         qCWarning(lcOcs) << "Could not parse reply to"
                          << _verb
                          << Utility::concatUrlPath(account()->url(), path())
                          << _params
                          << error.errorString()
                          << ":" << replyData;
+    } else {
+        statusCode  = getJsonReturnCode(json, message);
     }
 
-    QString message;
-    const int statusCode = getJsonReturnCode(json, message);
+    //... then it checks for the statusCode
     if (!_passStatusCodes.contains(statusCode)) {
         qCWarning(lcOcs) << "Reply to"
                          << _verb
@@ -120,8 +131,13 @@ bool OcsJob::finished()
                          << _params
                          << "has unexpected status code:" << statusCode << replyData;
         emit ocsError(statusCode, message);
+
     } else {
-        emit jobFinished(json);
+        // save new ETag value
+        if(reply()->rawHeaderList().contains("ETag"))
+            emit etagResponseHeaderReceived(reply()->rawHeader("ETag"), statusCode);
+
+        emit jobFinished(json, statusCode);
     }
     return true;
 }
diff --git a/src/gui/ocsjob.h b/src/gui/ocsjob.h
index 4de4d701e..36b580a71 100644
--- a/src/gui/ocsjob.h
+++ b/src/gui/ocsjob.h
@@ -26,6 +26,8 @@
 #define OCS_SUCCESS_STATUS_CODE 100
 // Apparantly the v2.php URLs can return that
 #define OCS_SUCCESS_STATUS_CODE_V2 200
+// not modified when using  ETag
+#define OCS_NOT_MODIFIED_STATUS_CODE_V2 304
 
 class QJsonDocument;
 
@@ -99,6 +101,14 @@ public:
      */
     static int getJsonReturnCode(const QJsonDocument &json, QString &message);
 
+    /**
+     * @brief Adds header to the request e.g. "If-None-Match"
+     * @param headerName a string with the header name
+     * @param value a string with the value
+     */
+    void addRawHeader(const QByteArray &headerName, const QByteArray &value);
+
+
 protected slots:
 
     /**
@@ -113,7 +123,7 @@ signals:
      *
      * @param reply the reply
      */
-    void jobFinished(QJsonDocument reply);
+    void jobFinished(QJsonDocument reply, int statusCode);
 
     /**
      * The status code was not one of the expected (passing)
@@ -124,6 +134,14 @@ signals:
      */
     void ocsError(int statusCode, const QString &message);
 
+    /**
+     * @brief etagResponseHeaderReceived - signal to report the ETag response header value
+     * from ocs api v2
+     * @param value - the ETag response header value
+     * @param statusCode - the OCS status code: 100 (!) for success
+     */
+    void etagResponseHeaderReceived(const QByteArray &value, int statusCode);
+
 private slots:
     virtual bool finished() Q_DECL_OVERRIDE;
 
@@ -131,6 +149,7 @@ private:
     QByteArray _verb;
     QList<QPair<QString, QString>> _params;
     QVector<int> _passStatusCodes;
+    QNetworkRequest _request;
 };
 }
 
diff --git a/src/gui/ocsnavigationappsjob.cpp b/src/gui/ocsnavigationappsjob.cpp
index b070873d5..db9ebf43b 100644
--- a/src/gui/ocsnavigationappsjob.cpp
+++ b/src/gui/ocsnavigationappsjob.cpp
@@ -30,9 +30,8 @@ void OcsNavigationAppsJob::getNavigationApps()
     start();
 }
 
-void OcsNavigationAppsJob::jobDone(const QJsonDocument &reply)
+void OcsNavigationAppsJob::jobDone(const QJsonDocument &reply, int statusCode)
 {
-
-    emit appsJobFinished(reply);
+    emit appsJobFinished(reply, statusCode);
 }
 }
diff --git a/src/gui/ocsnavigationappsjob.h b/src/gui/ocsnavigationappsjob.h
index 2c0c7bd4a..8c00c15d9 100644
--- a/src/gui/ocsnavigationappsjob.h
+++ b/src/gui/ocsnavigationappsjob.h
@@ -43,11 +43,12 @@ signals:
      * Result of the OCS request
      *
      * @param reply The reply
+     * @param statusCode the status code of the response
      */
-    void appsJobFinished(const QJsonDocument &reply);
+    void appsJobFinished(const QJsonDocument &reply, int statusCode);
 
 private slots:
-    void jobDone(const QJsonDocument &reply);
+    void jobDone(const QJsonDocument &reply, int statusCode);
 };
 }
 
diff --git a/src/gui/owncloudgui.cpp b/src/gui/owncloudgui.cpp
index 54f541a99..ff7438252 100644
--- a/src/gui/owncloudgui.cpp
+++ b/src/gui/owncloudgui.cpp
@@ -740,59 +740,82 @@ void ownCloudGui::setupActions()
     }
 }
 
+void ownCloudGui::slotEtagResponseHeaderReceived(const QByteArray &value, int statusCode){
+    if(statusCode == 200){
+        qCDebug(lcApplication) << "New navigation apps ETag Response Header received " << value;
+        auto account = qvariant_cast<AccountStatePtr>(sender()->property(propertyAccountC));
+        account->setNavigationAppsEtagResponseHeader(value);
+    }
+}
+
 void ownCloudGui::fetchNavigationApps(AccountStatePtr account, QMenu *accountMenu){
     OcsNavigationAppsJob *job = new OcsNavigationAppsJob(account->account());
-    job->setProperty(propertyAccountC, QVariant::fromValue(account->account()));
+    job->setProperty(propertyAccountC, QVariant::fromValue(account));
     job->setProperty(propertyMenuC, QVariant::fromValue(accountMenu));
+    job->addRawHeader("If-None-Match", account->navigationAppsEtagResponseHeader());
     connect(job, &OcsNavigationAppsJob::appsJobFinished, this, &ownCloudGui::slotNavigationAppsFetched);
+    connect(job, &OcsNavigationAppsJob::etagResponseHeaderReceived, this, &ownCloudGui::slotEtagResponseHeaderReceived);
     connect(job, &OcsNavigationAppsJob::ocsError, this, &ownCloudGui::slotOcsError);
     job->getNavigationApps();
 }
 
-void ownCloudGui::slotNavigationAppsFetched(const QJsonDocument &reply)
-{
-    if(!reply.isEmpty()){
-        auto element = reply.object().value("ocs").toObject().value("data");
-        auto navLinks = element.toArray();
-        if(navLinks.size() > 0){
-            if(auto account = qvariant_cast<AccountPtr>(sender()->property(propertyAccountC))){
-                if(QMenu *accountMenu = qvariant_cast<QMenu*>(sender()->property(propertyMenuC))){
-
-                    // when there is only one account add the nav links above the settings
-                    QAction *actionBefore = _actionSettings;
-
-                    // when there is more than one account add the nav links above pause/unpause folder or logout action
-                    if(AccountManager::instance()->accounts().size() > 1){
-                        foreach(QAction *action, accountMenu->actions()){
-
-                            // pause/unpause folder and logout actions have propertyAccountC
-                            if(auto actionAccount = qvariant_cast<AccountStatePtr>(action->property(propertyAccountC))){
-                                if(actionAccount->account() == account){
-                                    actionBefore = action;
-                                    break;
-                                }
-                             }
-                        }
-                    }
+void ownCloudGui::buildNavigationAppsMenu(AccountStatePtr account, QMenu *accountMenu){
+    auto navLinks = _navApps.value(account);
+    if(navLinks.size() > 0){
 
-                    // Create submenu with links
-                    QMenu *navLinksMenu = new QMenu(tr("Apps"));
-                    accountMenu->insertSeparator(actionBefore);
-                    accountMenu->insertMenu(actionBefore, navLinksMenu);
-                    foreach (const QJsonValue &value, navLinks) {
-                        auto navLink = value.toObject();
-                        QAction *action = new QAction(navLink.value("name").toString(), this);
-                        QUrl href(navLink.value("href").toString());
-                        connect(action, &QAction::triggered, this, [href] { QDesktopServices::openUrl(href); });
-                        navLinksMenu->addAction(action);
+        // when there is only one account add the nav links above the settings
+        QAction *actionBefore = _actionSettings;
+
+        // when there is more than one account add the nav links above pause/unpause folder or logout action
+        if(AccountManager::instance()->accounts().size() > 1){
+            foreach(QAction *action, accountMenu->actions()){
+
+                // pause/unpause folder and logout actions have propertyAccountC
+                if(auto actionAccount = qvariant_cast<AccountStatePtr>(action->property(propertyAccountC))){
+                    if(actionAccount == account){
+                        actionBefore = action;
+                        break;
                     }
-                    accountMenu->insertSeparator(actionBefore);
                 }
             }
         }
+
+        // Create submenu with links
+        QMenu *navLinksMenu = new QMenu(tr("Apps"));
+        accountMenu->insertSeparator(actionBefore);
+        accountMenu->insertMenu(actionBefore, navLinksMenu);
+        foreach (const QJsonValue &value, navLinks) {
+            auto navLink = value.toObject();
+            QAction *action = new QAction(navLink.value("name").toString(), this);
+            QUrl href(navLink.value("href").toString());
+            connect(action, &QAction::triggered, this, [href] { QDesktopServices::openUrl(href); });
+            navLinksMenu->addAction(action);
+        }
+        accountMenu->insertSeparator(actionBefore);
     }
 }
 
+void ownCloudGui::slotNavigationAppsFetched(const QJsonDocument &reply, int statusCode)
+{
+    auto account = qvariant_cast<AccountStatePtr>(sender()->property(propertyAccountC));
+    auto accountMenu = qvariant_cast<QMenu*>(sender()->property(propertyMenuC));
+
+    if (statusCode == 304) {
+        qCWarning(lcApplication) << "Status code " << statusCode << " Not Modified - No new navigation apps.";
+    } else {
+        if(!reply.isEmpty()){
+            auto element = reply.object().value("ocs").toObject().value("data");
+            auto navLinks = element.toArray();
+            if(account){
+                _navApps.insert(account, navLinks);
+            }
+         }
+    }
+
+    if(accountMenu)
+        buildNavigationAppsMenu(account, accountMenu);
+}
+
 void ownCloudGui::slotOcsError(int statusCode, const QString &message)
 {
     emit serverError(statusCode, message);
diff --git a/src/gui/owncloudgui.h b/src/gui/owncloudgui.h
index 432a22014..c855c5c74 100644
--- a/src/gui/owncloudgui.h
+++ b/src/gui/owncloudgui.h
@@ -93,7 +93,9 @@ public slots:
     void slotOpenPath(const QString &path);
     void slotAccountStateChanged();
     void slotTrayMessageIfServerUnsupported(Account *account);
-    void slotNavigationAppsFetched(const QJsonDocument &reply);
+    void slotNavigationAppsFetched(const QJsonDocument &reply, int statusCode);
+    void slotEtagResponseHeaderReceived(const QByteArray &value, int statusCode);
+
 
     /**
      * Open a share dialog for a file or folder.
@@ -121,6 +123,7 @@ private:
     void setupActions();
     void addAccountContextMenu(AccountStatePtr accountState, QMenu *menu, bool separateMenu);
     void fetchNavigationApps(AccountStatePtr account, QMenu *accountMenu);
+    void buildNavigationAppsMenu(AccountStatePtr account, QMenu *accountMenu);
 
     QPointer<Systray> _tray;
 #if defined(Q_OS_MAC)
@@ -155,6 +158,8 @@ private:
     QAction *_actionQuit;
     QAction *_actionCrash;
 
+    QMap<AccountStatePtr, QJsonArray> _navApps;
+
     QList<QAction *> _recentItemsActions;
     Application *_app;
 };