Simplify KNSBackend fetch logic

This code removes the custom pagination in KNSBackend in favour
of using the internal pagination in KNSCore::Engine. It further
removes the explicit call to request data, as this is already
done by setting the search term, which caused duplicated
results to be returned. Further, remove results already
returned when KNS requests the view to be cleared, further
reducing duplicate view entries.

nb: The fact setting the search term starts a new search is
undocumented, which will need fixing (some thorough
documentation work is ongoing in kns as part of a gsoc project)

BUG: 380138
Differential Revision: https://phabricator.kde.org/D6191
parent d5b12a6b
......@@ -31,6 +31,7 @@
#include <attica/providermanager.h>
// KDE includes
#include <knewstuffcore_version.h>
#include <KNSCore/Engine>
#include <KNSCore/QuestionManager>
#include <KConfigGroup>
......@@ -81,7 +82,6 @@ KNSBackend::KNSBackend(QObject* parent, const QString& iconName, const QString &
: AbstractResourcesBackend(parent)
, m_fetching(false)
, m_isValid(true)
, m_page(0)
, m_reviews(new KNSReviews(this))
, m_name(knsrc)
, m_iconName(iconName)
......@@ -107,6 +107,9 @@ KNSBackend::KNSBackend(QObject* parent, const QString& iconName, const QString &
m_engine = new KNSCore::Engine(this);
m_engine->init(m_name);
#if KNEWSTUFFCORE_VERSION_MAJOR==5 && KNEWSTUFFCORE_VERSION_MAJOR>=36
m_engine->setPageSize(100);
#endif
// Setting setFetching to false when we get an error ensures we don't end up in an eternally-fetching state
connect(m_engine, &KNSCore::Engine::signalError, this, [this](const QString &error) {
if(error == QLatin1Literal("All categories are missing")) {
......@@ -121,8 +124,17 @@ KNSBackend::KNSBackend(QObject* parent, const QString& iconName, const QString &
connect(m_engine, &KNSCore::Engine::signalEntriesLoaded, this, &KNSBackend::receivedEntries, Qt::QueuedConnection);
connect(m_engine, &KNSCore::Engine::signalEntryChanged, this, &KNSBackend::statusChanged);
connect(m_engine, &KNSCore::Engine::signalEntryDetailsLoaded, this, &KNSBackend::statusChanged);
m_page = -1;
connect(m_engine, &KNSCore::Engine::signalProvidersLoaded, m_engine, &KNSCore::Engine::checkForInstalled);
connect(m_engine, &KNSCore::Engine::signalResetView, this, [this](){
// If KNS tells us we should reset the view, what that means here is to remove
// references to all the resources we've already told the agregator model about
// from the model, as they will be added again...
foreach(AbstractResource* res, m_resourcesByName.values()) {
resourceRemoved(res);
res->deleteLater();
}
m_resourcesByName.clear();
});
m_responsePending = true;
const QVector<QPair<FilterType, QString>> filters = { {CategoryFilter, fileName } };
......@@ -191,20 +203,19 @@ void KNSBackend::receivedEntries(const KNSCore::EntryInternal::List& entries)
Q_EMIT receivedResources(resources);
}
if(resources.isEmpty() || m_page < 0) {
if(resources.isEmpty()) {
Q_EMIT searchFinished();
Q_EMIT availableForQueries();
setFetching(false);
return;
}
// qDebug() << "received" << objectName() << this << m_page << m_resourcesByName.count();
// qDebug() << "received" << objectName() << this << m_resourcesByName.count();
if (!m_responsePending) {
++m_page;
// We _have_ to set this first. If we do not, we may run into a situation where the
// data request will conclude immediately, causing m_responsePending to remain true
// for perpetuity as the slots will be called before the function returns.
m_responsePending = true;
m_engine->requestData(m_page, 100);
m_engine->requestMoreData();
} else {
Q_EMIT availableForQueries();
}
......@@ -327,14 +338,13 @@ ResultsStream * KNSBackend::searchStream(const QString &searchText)
Q_EMIT startingSearch();
auto stream = new ResultsStream(QStringLiteral("KNS-search-")+name());
auto start = [this, stream, searchText]() {
connect(this, &KNSBackend::receivedResources, stream, &ResultsStream::resourcesFound);
connect(this, &KNSBackend::searchFinished, stream, &ResultsStream::finish);
connect(this, &KNSBackend::startingSearch, stream, &ResultsStream::finish);
auto start = [this, searchText]() {
// No need to explicitly launch a search, setting the search term already does that for us
m_engine->setSearchTerm(searchText);
m_engine->requestData(0, 100);
m_responsePending = true;
m_page = 0;
connect(this, &KNSBackend::receivedResources, stream, &ResultsStream::resourcesFound);
connect(this, &KNSBackend::searchFinished, stream, &ResultsStream::finish);
connect(this, &KNSBackend::startingSearch, stream, &ResultsStream::finish);
};
if (m_responsePending) {
connect(this, &KNSBackend::availableForQueries, stream, start, Qt::QueuedConnection);
......
......@@ -87,7 +87,6 @@ private:
bool m_isValid;
KNSCore::Engine* m_engine;
QHash<QString, AbstractResource*> m_resourcesByName;
int m_page;
KNSReviews* const m_reviews;
QString m_name;
QString m_iconName;
......
......@@ -38,6 +38,10 @@ StandardBackendUpdater::StandardBackendUpdater(AbstractResourcesBackend* parent)
{
connect(m_backend, &AbstractResourcesBackend::fetchingChanged, this, &StandardBackendUpdater::refreshUpdateable);
connect(m_backend, &AbstractResourcesBackend::resourcesChanged, this, &StandardBackendUpdater::resourcesChanged);
connect(m_backend, &AbstractResourcesBackend::resourceRemoved, this, [this](AbstractResource* resource){
m_upgradeable.remove(resource);
m_toUpgrade.remove(resource);
});
connect(TransactionModel::global(), &TransactionModel::transactionRemoved, this, &StandardBackendUpdater::transactionRemoved);
connect(TransactionModel::global(), &TransactionModel::transactionAdded, this, &StandardBackendUpdater::transactionAdded);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment