From kde-commits Fri Mar 31 21:56:05 2017 From: Thomas Fischer Date: Fri, 31 Mar 2017 21:56:05 +0000 To: kde-commits Subject: [kbibtex/kbibtex/0.7] src/networking/zotero: Improving handling Zotero's 'Backoff'/'Retry-After' hea Message-Id: X-MARC-Message: https://marc.info/?l=kde-commits&m=149099738115491 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 =3D QDateTime::currentDateTime().addSecs(dura= tion + 1); emit backoffModeStart(); /// Use single-shot timer and functor to emit signal diff --git a/src/networking/zotero/collection.cpp b/src/networking/zotero/c= ollection.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 =3D downloadQueue.dequeue(); KUrl url =3D 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 =3D url; + QTimer::singleShot((api->backoffSecondsLeft() + 1) * 1000,= p, SLOT(singleShotRequestZoteroUrl())); + } else + requestZoteroUrl(url); } else { initialized =3D true; p->emitFinishedLoading(); @@ -92,6 +97,7 @@ Collection::Collection(QSharedPointer api, Q= Object *parent) KUrl url =3D 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 =3D url; QTimer::singleShot((api->backoffSecondsLeft() + 1) * 1000, this, S= LOT(singleShotRequestZoteroUrl())); } else @@ -163,10 +169,17 @@ void Collection::finishedFetchingCollection() QNetworkReply *reply =3D static_cast(sender()); QString parentId =3D 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-A= fter").constData()).toInt()); + if (reply->hasRawHeader("Backoff")) { + bool ok =3D false; + int time =3D QString::fromLatin1(reply->rawHeader("Backoff").const= Data()).toInt(&ok); + if (!ok) time =3D 10; ///< parsing argument of raw header 'Backoff= ' failed? 10 seconds is fallback + d->api->startBackoff(time); + } else if (reply->hasRawHeader("Retry-After")) { + bool ok =3D false; + int time =3D QString::fromLatin1(reply->rawHeader("Retry-After").c= onstData()).toInt(&ok); + if (!ok) time =3D 10; ///< parsing argument of raw header 'Retry-A= fter' failed? 10 seconds is fallback + d->api->startBackoff(time); + } = if (reply->error() =3D=3D QNetworkReply::NoError) { QString nextPage; @@ -214,7 +227,12 @@ void Collection::finishedFetchingCollection() } = if (!nextPage.isEmpty()) { - d->requestZoteroUrl(nextPage); + if (d->api->inBackoffMode() && d->queuedRequestZoteroUrl.isEmp= ty()) { + /// If Zotero asked to 'back off', wait until this period = is over before issuing the next request + d->queuedRequestZoteroUrl =3D nextPage; + QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * 10= 00, this, SLOT(singleShotRequestZoteroUrl())); + } else + d->requestZoteroUrl(nextPage); } else d->runNextInDownloadQueue(); } else { diff --git a/src/networking/zotero/groups.cpp b/src/networking/zotero/group= s.cpp index e0d3a59a..b725f9e8 100644 --- a/src/networking/zotero/groups.cpp +++ b/src/networking/zotero/groups.cpp @@ -67,6 +67,7 @@ Groups::Groups(QSharedPointer api, QObject *= parent) Q_ASSERT_X(url.path().contains(QLatin1String("users/")), "Groups::Grou= ps(QSharedPointer api, QObject *parent)", "Provided base URL d= oes 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 =3D url; QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * 1000, this= , SLOT(singleShotRequestZoteroUrl())); } else @@ -97,10 +98,17 @@ void Groups::finishedFetchingGroups() { QNetworkReply *reply =3D static_cast(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-A= fter").constData()).toInt()); + if (reply->hasRawHeader("Backoff")) { + bool ok =3D false; + int time =3D QString::fromLatin1(reply->rawHeader("Backoff").const= Data()).toInt(&ok); + if (!ok) time =3D 10; ///< parsing argument of raw header 'Backoff= ' failed? 10 seconds is fallback + d->api->startBackoff(time); + } else if (reply->hasRawHeader("Retry-After")) { + bool ok =3D false; + int time =3D QString::fromLatin1(reply->rawHeader("Retry-After").c= onstData()).toInt(&ok); + if (!ok) time =3D 10; ///< parsing argument of raw header 'Retry-A= fter' failed? 10 seconds is fallback + d->api->startBackoff(time); + } = if (reply->error() =3D=3D QNetworkReply::NoError) { QString nextPage; @@ -133,7 +141,12 @@ void Groups::finishedFetchingGroups() } = if (!nextPage.isEmpty()) - d->requestZoteroUrl(nextPage); + if (d->api->inBackoffMode() && d->queuedRequestZoteroUrl.isEmp= ty()) { + /// If Zotero asked to 'back off', wait until this period = is over before issuing the next request + d->queuedRequestZoteroUrl =3D nextPage; + QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * 10= 00, this, SLOT(singleShotRequestZoteroUrl())); + } else + d->requestZoteroUrl(nextPage); else { d->busy =3D false; d->initialized =3D 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 o= ver before issuing the next request + queuedRequestZoteroUrl =3D internalUrl; + QTimer::singleShot((api->backoffSecondsLeft() + 1) * 1000, p, = SLOT(singleShotRequestZoteroUrl())); + } else + requestZoteroUrl(internalUrl); } }; = @@ -87,6 +92,7 @@ void Items::retrieveItemsByCollection(const QString &coll= ection) url.addPath(QString(QLatin1String("/collections/%1/items")).arg(co= llection)); 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 =3D 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 =3D url; QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * 1000, this= , SLOT(singleShotRequestZoteroUrl())); } else @@ -114,10 +121,17 @@ void Items::finishedFetchingItems() bool ok =3D false; const int start =3D 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-A= fter").constData()).toInt()); + if (reply->hasRawHeader("Backoff")) { + bool ok =3D false; + int time =3D QString::fromLatin1(reply->rawHeader("Backoff").const= Data()).toInt(&ok); + if (!ok) time =3D 10; ///< parsing argument of raw header 'Backoff= ' failed? 10 seconds is fallback + d->api->startBackoff(time); + } else if (reply->hasRawHeader("Retry-After")) { + bool ok =3D false; + int time =3D QString::fromLatin1(reply->rawHeader("Retry-After").c= onstData()).toInt(&ok); + if (!ok) time =3D 10; ///< parsing argument of raw header 'Retry-A= fter' failed? 10 seconds is fallback + d->api->startBackoff(time); + } = if (reply->error() =3D=3D QNetworkReply::NoError && ok) { const QString bibTeXcode =3D QString::fromUtf8(reply->readAll().co= nstData()); 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 api, QObject *pare= nt) 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 =3D url; QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * 1000, this= , SLOT(singleShotRequestZoteroUrl())); } else @@ -98,10 +99,17 @@ void Tags::finishedFetchingTags() { QNetworkReply *reply =3D static_cast(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-A= fter").constData()).toInt()); + if (reply->hasRawHeader("Backoff")) { + bool ok =3D false; + int time =3D QString::fromLatin1(reply->rawHeader("Backoff").const= Data()).toInt(&ok); + if (!ok) time =3D 10; ///< parsing argument of raw header 'Backoff= ' failed? 10 seconds is fallback + d->api->startBackoff(time); + } else if (reply->hasRawHeader("Retry-After")) { + bool ok =3D false; + int time =3D QString::fromLatin1(reply->rawHeader("Retry-After").c= onstData()).toInt(&ok); + if (!ok) time =3D 10; ///< parsing argument of raw header 'Retry-A= fter' failed? 10 seconds is fallback + d->api->startBackoff(time); + } = if (reply->error() =3D=3D 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.isEmp= ty()) { + /// If Zotero asked to 'back off', wait until this period = is over before issuing the next request + d->queuedRequestZoteroUrl =3D nextPage; + QTimer::singleShot((d->api->backoffSecondsLeft() + 1) * 10= 00, this, SLOT(singleShotRequestZoteroUrl())); + } else + d->requestZoteroUrl(nextPage); + } else { d->busy =3D false; d->initialized =3D true; emit finishedLoading();