[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kbibtex/kbibtex/0.7] src/networking/zotero: Improving handling Zotero's 'Backoff'/'Retry-After' hea
From:       Thomas Fischer <null () kde ! org>
Date:       2017-03-31 21:56:05
Message-ID: E1cu4Wb-0004F6-CT () code ! kde ! org
[Download RAW message or body]

Git commit f2b95ddb64bbd0c101ab7ad4d3cc4d7211c096c2 by Thomas Fischer.
Committed on 31/03/2017 at 21:24.
Pushed by thomasfischer into branch 'kbibtex/0.7'.

Improving handling Zotero's 'Backoff'/'Retry-After' headers

... by checking the current back-off time before a new rquest and
correctly parsing HTTP headers for 'Backoff' and 'Retry-After',
respectively.

Due to lack of test cases, the current code can only be assumed to
work, but has not been tested or verified.

Manual backport of commit f94fb1453d1cb4c745 from branch 'master'.

M  +1    -1    src/networking/zotero/api.cpp
M  +24   -6    src/networking/zotero/collection.cpp
M  +18   -5    src/networking/zotero/groups.cpp
M  +19   -5    src/networking/zotero/items.cpp
M  +20   -7    src/networking/zotero/tags.cpp

https://commits.kde.org/kbibtex/f2b95ddb64bbd0c101ab7ad4d3cc4d7211c096c2

diff --git a/src/networking/zotero/api.cpp b/src/networking/zotero/api.cpp
index eedf123d..ed60499e 100644
--- a/src/networking/zotero/api.cpp
+++ b/src/networking/zotero/api.cpp
@@ -95,7 +95,7 @@ QNetworkRequest API::request(const KUrl &url) const
 }
 
 void API::startBackoff(int duration) {
-    if (duration > 0) {
+    if (duration > 0 && !inBackoffMode()) {
         d->backoffElapseTime = QDateTime::currentDateTime().addSecs(duration + 1);
         emit backoffModeStart();
         /// Use single-shot timer and functor to emit signal
diff --git a/src/networking/zotero/collection.cpp \
b/src/networking/zotero/collection.cpp index 4b258605..380a0633 100644
--- a/src/networking/zotero/collection.cpp
+++ b/src/networking/zotero/collection.cpp
@@ -74,7 +74,12 @@ public:
             const QString head = downloadQueue.dequeue();
             KUrl url = api->baseUrl();
             url.addPath(QString(QLatin1String("/collections/%1/collections")).arg(head));
                
-            requestZoteroUrl(url);
+            if (api->inBackoffMode() && queuedRequestZoteroUrl.isEmpty()) {
+                /// If Zotero asked to 'back off', wait until this period is over \
before issuing the next request +                queuedRequestZoteroUrl = url;
+                QTimer::singleShot((api->backoffSecondsLeft() + 1) * 1000, p, \
SLOT(singleShotRequestZoteroUrl())); +            } else
+                requestZoteroUrl(url);
         } else {
             initialized = true;
             p->emitFinishedLoading();
@@ -92,6 +97,7 @@ Collection::Collection(QSharedPointer<Zotero::API> api, QObject \
*parent)  KUrl url = api->baseUrl();
     url.addPath(QLatin1String("/collections/top"));
     if (api->inBackoffMode() && d->queuedRequestZoteroUrl.isEmpty()) {
+        /// If Zotero asked to 'back off', wait until this period is over before \
issuing the next request  d->queuedRequestZoteroUrl = url;
         QTimer::singleShot((api->backoffSecondsLeft() + 1) * 1000, this, \
SLOT(singleShotRequestZoteroUrl()));  } else
@@ -163,10 +169,17 @@ void Collection::finishedFetchingCollection()
     QNetworkReply *reply = static_cast<QNetworkReply *>(sender());
     QString parentId = Private::top;
 
-    if (reply->hasRawHeader("Backoff"))
-        d->api->startBackoff(QString::fromLatin1(reply->rawHeader("Backoff").constData()).toInt());
                
-    else if (reply->hasRawHeader("Retry-After"))
-        d->api->startBackoff(QString::fromLatin1(reply->rawHeader("Retry-After").constData()).toInt());
 +    if (reply->hasRawHeader("Backoff")) {
+        bool ok = false;
+        int time = QString::fromLatin1(reply->rawHeader("Backoff").constData()).toInt(&ok);
 +        if (!ok) time = 10; ///< parsing argument of raw header 'Backoff' failed? \
10 seconds is fallback +        d->api->startBackoff(time);
+    } else if (reply->hasRawHeader("Retry-After")) {
+        bool ok = false;
+        int time = QString::fromLatin1(reply->rawHeader("Retry-After").constData()).toInt(&ok);
 +        if (!ok) time = 10; ///< parsing argument of raw header 'Retry-After' \
failed? 10 seconds is fallback +        d->api->startBackoff(time);
+    }
 
     if (reply->error() == QNetworkReply::NoError) {
         QString nextPage;
@@ -214,7 +227,12 @@ void Collection::finishedFetchingCollection()
         }
 
         if (!nextPage.isEmpty()) {
-            d->requestZoteroUrl(nextPage);
+            if (d->api->inBackoffMode() && d->queuedRequestZoteroUrl.isEmpty()) {
+                /// If Zotero asked to 'back off', wait until this period is over \
before issuing the next request +                d->queuedRequestZoteroUrl = \
nextPage; +                QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * \
1000, this, SLOT(singleShotRequestZoteroUrl())); +            } else
+                d->requestZoteroUrl(nextPage);
         } else
             d->runNextInDownloadQueue();
     } else {
diff --git a/src/networking/zotero/groups.cpp b/src/networking/zotero/groups.cpp
index e0d3a59a..b725f9e8 100644
--- a/src/networking/zotero/groups.cpp
+++ b/src/networking/zotero/groups.cpp
@@ -67,6 +67,7 @@ Groups::Groups(QSharedPointer<Zotero::API> api, QObject *parent)
     Q_ASSERT_X(url.path().contains(QLatin1String("users/")), \
"Groups::Groups(QSharedPointer<Zotero::API> api, QObject *parent)", "Provided base \
URL does not contain 'users/' as expected");  url.addPath(QLatin1String("/groups"));
     if (d->api->inBackoffMode() && d->queuedRequestZoteroUrl.isEmpty()) {
+        /// If Zotero asked to 'back off', wait until this period is over before \
issuing the next request  d->queuedRequestZoteroUrl = url;
         QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * 1000, this, \
SLOT(singleShotRequestZoteroUrl()));  } else
@@ -97,10 +98,17 @@ void Groups::finishedFetchingGroups()
 {
     QNetworkReply *reply = static_cast<QNetworkReply *>(sender());
 
-    if (reply->hasRawHeader("Backoff"))
-        d->api->startBackoff(QString::fromLatin1(reply->rawHeader("Backoff").constData()).toInt());
                
-    else if (reply->hasRawHeader("Retry-After"))
-        d->api->startBackoff(QString::fromLatin1(reply->rawHeader("Retry-After").constData()).toInt());
 +    if (reply->hasRawHeader("Backoff")) {
+        bool ok = false;
+        int time = QString::fromLatin1(reply->rawHeader("Backoff").constData()).toInt(&ok);
 +        if (!ok) time = 10; ///< parsing argument of raw header 'Backoff' failed? \
10 seconds is fallback +        d->api->startBackoff(time);
+    } else if (reply->hasRawHeader("Retry-After")) {
+        bool ok = false;
+        int time = QString::fromLatin1(reply->rawHeader("Retry-After").constData()).toInt(&ok);
 +        if (!ok) time = 10; ///< parsing argument of raw header 'Retry-After' \
failed? 10 seconds is fallback +        d->api->startBackoff(time);
+    }
 
     if (reply->error() == QNetworkReply::NoError) {
         QString nextPage;
@@ -133,7 +141,12 @@ void Groups::finishedFetchingGroups()
         }
 
         if (!nextPage.isEmpty())
-            d->requestZoteroUrl(nextPage);
+            if (d->api->inBackoffMode() && d->queuedRequestZoteroUrl.isEmpty()) {
+                /// If Zotero asked to 'back off', wait until this period is over \
before issuing the next request +                d->queuedRequestZoteroUrl = \
nextPage; +                QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * \
1000, this, SLOT(singleShotRequestZoteroUrl())); +            } else
+                d->requestZoteroUrl(nextPage);
         else {
             d->busy = false;
             d->initialized = true;
diff --git a/src/networking/zotero/items.cpp b/src/networking/zotero/items.cpp
index 1eb3a69d..a0924ff7 100644
--- a/src/networking/zotero/items.cpp
+++ b/src/networking/zotero/items.cpp
@@ -63,7 +63,12 @@ public:
         internalUrl.removeQueryItem(queryItemStart);
         internalUrl.addQueryItem(queryItemStart, QString::number(start));
 
-        requestZoteroUrl(internalUrl);
+        if (api->inBackoffMode() && queuedRequestZoteroUrl.isEmpty()) {
+            /// If Zotero asked to 'back off', wait until this period is over before \
issuing the next request +            queuedRequestZoteroUrl = internalUrl;
+            QTimer::singleShot((api->backoffSecondsLeft() + 1) * 1000, p, \
SLOT(singleShotRequestZoteroUrl())); +        } else
+            requestZoteroUrl(internalUrl);
     }
 };
 
@@ -87,6 +92,7 @@ void Items::retrieveItemsByCollection(const QString &collection)
         url.addPath(QString(QLatin1String("/collections/%1/items")).arg(collection));
  url.addQueryItem(QLatin1String("format"), QLatin1String("bibtex"));
     if (d->api->inBackoffMode() && d->queuedRequestZoteroUrl.isEmpty()) {
+        /// If Zotero asked to 'back off', wait until this period is over before \
issuing the next request  d->queuedRequestZoteroUrl = url;
         QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * 1000, this, \
SLOT(singleShotRequestZoteroUrl()));  } else
@@ -101,6 +107,7 @@ void  Items::retrieveItemsByTag(const QString &tag)
     url.addPath(QLatin1String("items"));
     url.addQueryItem(QLatin1String("format"), QLatin1String("bibtex"));
     if (d->api->inBackoffMode() && d->queuedRequestZoteroUrl.isEmpty()) {
+        /// If Zotero asked to 'back off', wait until this period is over before \
issuing the next request  d->queuedRequestZoteroUrl = url;
         QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * 1000, this, \
SLOT(singleShotRequestZoteroUrl()));  } else
@@ -114,10 +121,17 @@ void Items::finishedFetchingItems()
     bool ok = false;
     const int start = reply->url().queryItemValue(queryItemStart).toInt(&ok);
 
-    if (reply->hasRawHeader("Backoff"))
-        d->api->startBackoff(QString::fromLatin1(reply->rawHeader("Backoff").constData()).toInt());
                
-    else if (reply->hasRawHeader("Retry-After"))
-        d->api->startBackoff(QString::fromLatin1(reply->rawHeader("Retry-After").constData()).toInt());
 +    if (reply->hasRawHeader("Backoff")) {
+        bool ok = false;
+        int time = QString::fromLatin1(reply->rawHeader("Backoff").constData()).toInt(&ok);
 +        if (!ok) time = 10; ///< parsing argument of raw header 'Backoff' failed? \
10 seconds is fallback +        d->api->startBackoff(time);
+    } else if (reply->hasRawHeader("Retry-After")) {
+        bool ok = false;
+        int time = QString::fromLatin1(reply->rawHeader("Retry-After").constData()).toInt(&ok);
 +        if (!ok) time = 10; ///< parsing argument of raw header 'Retry-After' \
failed? 10 seconds is fallback +        d->api->startBackoff(time);
+    }
 
     if (reply->error() == QNetworkReply::NoError && ok) {
         const QString bibTeXcode = QString::fromUtf8(reply->readAll().constData());
diff --git a/src/networking/zotero/tags.cpp b/src/networking/zotero/tags.cpp
index f10110a1..3d750d7c 100644
--- a/src/networking/zotero/tags.cpp
+++ b/src/networking/zotero/tags.cpp
@@ -68,6 +68,7 @@ Tags::Tags(QSharedPointer<Zotero::API> api, QObject *parent)
     url.addPath(QLatin1String("/tags"));
 
     if (api->inBackoffMode() && d->queuedRequestZoteroUrl.isEmpty()) {
+        /// If Zotero asked to 'back off', wait until this period is over before \
issuing the next request  d->queuedRequestZoteroUrl = url;
         QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * 1000, this, \
SLOT(singleShotRequestZoteroUrl()));  } else
@@ -98,10 +99,17 @@ void Tags::finishedFetchingTags()
 {
     QNetworkReply *reply = static_cast<QNetworkReply *>(sender());
 
-    if (reply->hasRawHeader("Backoff"))
-        d->api->startBackoff(QString::fromLatin1(reply->rawHeader("Backoff").constData()).toInt());
                
-    else if (reply->hasRawHeader("Retry-After"))
-        d->api->startBackoff(QString::fromLatin1(reply->rawHeader("Retry-After").constData()).toInt());
 +    if (reply->hasRawHeader("Backoff")) {
+        bool ok = false;
+        int time = QString::fromLatin1(reply->rawHeader("Backoff").constData()).toInt(&ok);
 +        if (!ok) time = 10; ///< parsing argument of raw header 'Backoff' failed? \
10 seconds is fallback +        d->api->startBackoff(time);
+    } else if (reply->hasRawHeader("Retry-After")) {
+        bool ok = false;
+        int time = QString::fromLatin1(reply->rawHeader("Retry-After").constData()).toInt(&ok);
 +        if (!ok) time = 10; ///< parsing argument of raw header 'Retry-After' \
failed? 10 seconds is fallback +        d->api->startBackoff(time);
+    }
 
     if (reply->error() == QNetworkReply::NoError) {
         QString nextPage;
@@ -133,9 +141,14 @@ void Tags::finishedFetchingTags()
                 break;
         }
 
-        if (!nextPage.isEmpty())
-            d->requestZoteroUrl(nextPage);
-        else {
+        if (!nextPage.isEmpty()) {
+            if (d->api->inBackoffMode() && d->queuedRequestZoteroUrl.isEmpty()) {
+                /// If Zotero asked to 'back off', wait until this period is over \
before issuing the next request +                d->queuedRequestZoteroUrl = \
nextPage; +                QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * \
1000, this, SLOT(singleShotRequestZoteroUrl())); +            } else
+                d->requestZoteroUrl(nextPage);
+        } else {
             d->busy = false;
             d->initialized = true;
             emit finishedLoading();


[prev in list] [next in list] [prev in thread] [next in thread]