Commit a6469045 authored by David Faure's avatar David Faure
Browse files

Fix aggregation logic for cacheOnly and ignoreErrors.

Summary:
Add unittests for AggregatedFetch*Scope.
They show that subscribers really need to better clean up after
themselves when unregistering, that's for the next commit (this test is too
low-level for that).

Test Plan: new test

Reviewers: dvratil

Reviewed By: dvratil

Subscribers: kde-pim

Tags: #kde_pim

Differential Revision: https://phabricator.kde.org/D18670
parent 5c582e86
......@@ -87,6 +87,7 @@ add_server_test(itemretrievertest.cpp)
add_server_test(notificationsubscribertest.cpp)
add_server_test(parttypehelpertest.cpp)
add_server_test(collectionstatisticstest.cpp)
add_server_test(aggregatedfetchscopetest.cpp)
if (SQLITE_FOUND) # tests using the fake server need the QSQLITE3 plugin
add_server_test(partstreamertest.cpp)
......
/*
Copyright (c) 2019 David Faure <faure@kde.org>
This library is free software; you can redistribute it and/or modify it
under the terms of the GNU Library General Public License as published by
the Free Software Foundation; either version 2 of the License, or (at your
option) any later version.
This library is distributed in the hope that it will be useful, but WITHOUT
ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
FITNESS FOR A PARTICULAR PURPOSE. See the GNU Library General Public
License for more details.
You should have received a copy of the GNU Library General Public License
along with this library; see the file COPYING.LIB. If not, write to the
Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
02110-1301, USA.
*/
#include <QObject>
#include <shared/aktest.h>
#include "aggregatedfetchscope.h"
#include <QTest>
using namespace Akonadi;
using namespace Akonadi::Server;
class AggregatedFetchScopeTest : public QObject
{
Q_OBJECT
private Q_SLOTS:
void testTagApply()
{
AggregatedTagFetchScope scope;
// first subscriber, A
scope.addSubscriber();
Protocol::TagFetchScope oldTagScope, tagScopeA;
QSet<QByteArray> attrs = {"FOO"};
tagScopeA.setAttributes(attrs);
tagScopeA.setFetchIdOnly(true);
scope.apply(oldTagScope, tagScopeA);
QCOMPARE(scope.attributes(), attrs);
QVERIFY(scope.fetchIdOnly());
// second subscriber, B
Protocol::TagFetchScope tagScopeB = tagScopeA;
tagScopeB.setFetchIdOnly(false);
scope.addSubscriber();
scope.apply(oldTagScope, tagScopeB);
QCOMPARE(scope.attributes(), attrs);
QVERIFY(!scope.fetchIdOnly());
// then B goes away
scope.apply(tagScopeB, oldTagScope);
scope.removeSubscriber();
QCOMPARE(scope.attributes(), attrs);
QVERIFY(scope.fetchIdOnly());
// A goes away
scope.apply(tagScopeA, oldTagScope);
scope.removeSubscriber();
QCOMPARE(scope.attributes(), QSet<QByteArray>());
}
void testCollectionApply()
{
AggregatedCollectionFetchScope scope;
// first subscriber, A
scope.addSubscriber();
Protocol::CollectionFetchScope oldCollectionScope, collectionScopeA;
QSet<QByteArray> attrs = {"FOO"};
collectionScopeA.setAttributes(attrs);
collectionScopeA.setFetchIdOnly(true);
scope.apply(oldCollectionScope, collectionScopeA);
QCOMPARE(scope.attributes(), attrs);
QVERIFY(scope.fetchIdOnly());
// second subscriber, B
Protocol::CollectionFetchScope collectionScopeB = collectionScopeA;
collectionScopeB.setFetchIdOnly(false);
scope.addSubscriber();
scope.apply(oldCollectionScope, collectionScopeB);
QCOMPARE(scope.attributes(), attrs);
QVERIFY(!scope.fetchIdOnly());
// then B goes away
scope.apply(collectionScopeB, oldCollectionScope);
scope.removeSubscriber();
QCOMPARE(scope.attributes(), attrs);
QVERIFY(scope.fetchIdOnly());
// A goes away
scope.apply(collectionScopeA, oldCollectionScope);
scope.removeSubscriber();
QCOMPARE(scope.attributes(), QSet<QByteArray>());
}
void testItemApply()
{
AggregatedItemFetchScope scope;
QCOMPARE(scope.ancestorDepth(), Protocol::ItemFetchScope::NoAncestor);
// first subscriber, A
scope.addSubscriber();
Protocol::ItemFetchScope oldItemScope, itemScopeA;
QVector<QByteArray> parts = {"FOO"};
QSet<QByteArray> partsSet = {"FOO"};
itemScopeA.setRequestedParts(parts);
itemScopeA.setAncestorDepth(Protocol::ItemFetchScope::ParentAncestor);
itemScopeA.setFetch(Protocol::ItemFetchScope::CacheOnly);
itemScopeA.setFetch(Protocol::ItemFetchScope::IgnoreErrors);
scope.apply(oldItemScope, itemScopeA);
QCOMPARE(scope.requestedParts(), partsSet);
QCOMPARE(scope.ancestorDepth(), Protocol::ItemFetchScope::ParentAncestor);
QVERIFY(scope.cacheOnly());
QVERIFY(scope.ignoreErrors());
// second subscriber, B
Protocol::ItemFetchScope itemScopeB = itemScopeA;
itemScopeB.setAncestorDepth(Protocol::ItemFetchScope::AllAncestors);
scope.addSubscriber();
QVERIFY(!scope.cacheOnly()); // they don't agree so: false
QVERIFY(!scope.ignoreErrors());
scope.apply(oldItemScope, itemScopeB);
QCOMPARE(scope.requestedParts(), partsSet);
QCOMPARE(scope.ancestorDepth(), Protocol::ItemFetchScope::AllAncestors);
// subscriber C with ParentAncestor - but that won't make change it
Protocol::ItemFetchScope itemScopeC = itemScopeA;
scope.addSubscriber();
scope.apply(oldItemScope, itemScopeC);
QCOMPARE(scope.requestedParts(), partsSet);
QCOMPARE(scope.ancestorDepth(), Protocol::ItemFetchScope::AllAncestors); // no change
// then C goes away
scope.apply(itemScopeC, oldItemScope);
scope.removeSubscriber();
QCOMPARE(scope.requestedParts(), partsSet);
QCOMPARE(scope.ancestorDepth(), Protocol::ItemFetchScope::AllAncestors);
// then B goes away
scope.apply(itemScopeB, oldItemScope);
scope.removeSubscriber();
QCOMPARE(scope.requestedParts(), partsSet);
QCOMPARE(scope.ancestorDepth(), Protocol::ItemFetchScope::ParentAncestor);
// A goes away
scope.apply(itemScopeA, oldItemScope);
scope.removeSubscriber();
QCOMPARE(scope.requestedParts(), QSet<QByteArray>());
QCOMPARE(scope.ancestorDepth(), Protocol::ItemFetchScope::NoAncestor);
}
};
QTEST_MAIN(AggregatedFetchScopeTest)
#include "aggregatedfetchscopetest.moc"
......@@ -116,6 +116,7 @@ public:
QHash<QByteArray, int> partsCount;
QSet<QByteArray> tags;
QHash<QByteArray, int> tagsCount;
int subscribers = 0;
int ancestors[3] = { 0, 0, 0 }; // 3 = size of AncestorDepth enum
int cacheOnly = 0;
int fullPayload = 0;
......@@ -363,7 +364,7 @@ bool AggregatedItemFetchScope::cacheOnly() const
LOCKED_D(const AggregatedItemFetchScope)
// Aggregation: we can return true only if everyone wants cached data only,
// otherwise there's at least one subscriber who wants uncached data
return d->cacheOnly == 0;
return d->cacheOnly == d->subscribers;
}
void AggregatedItemFetchScope::setCacheOnly(bool cacheOnly)
......@@ -444,7 +445,7 @@ bool AggregatedItemFetchScope::ignoreErrors() const
LOCKED_D(const AggregatedItemFetchScope)
// Aggregation: return true only if everyone wants to ignore errors, otherwise
// there's at least one subscriber who does not want to ignore them
return d->ignoreErrors == 0;
return d->ignoreErrors == d->subscribers;
}
void AggregatedItemFetchScope::setIgnoreErrors(bool ignoreErrors)
......@@ -531,7 +532,17 @@ void AggregatedItemFetchScope::setFetchVirtualReferences(bool fetchVRefs)
d->updateBool(fetchVRefs, d->fetchVRefs);
}
void AggregatedItemFetchScope::addSubscriber()
{
LOCKED_D(AggregatedItemFetchScope)
++d->subscribers;
}
void AggregatedItemFetchScope::removeSubscriber()
{
LOCKED_D(AggregatedItemFetchScope)
--d->subscribers;
}
......
......@@ -103,6 +103,9 @@ public:
bool fetchVirtualReferences() const;
void setFetchVirtualReferences(bool fetchVRefs);
void addSubscriber();
void removeSubscriber();
private:
AggregatedItemFetchScopePrivate * const d_ptr;
Q_DECLARE_PRIVATE(AggregatedItemFetchScope)
......
......@@ -49,6 +49,7 @@ NotificationSubscriber::NotificationSubscriber(NotificationManager *manager)
, mNotificationDebugging(false)
{
if (mManager) {
mManager->itemFetchScope()->addSubscriber();
mManager->collectionFetchScope()->addSubscriber();
mManager->tagFetchScope()->addSubscriber();
}
......@@ -172,6 +173,9 @@ void NotificationSubscriber::disconnectSubscriber()
auto tfs = mManager->tagFetchScope();
tfs->removeSubscriber();
auto ifs = mManager->itemFetchScope();
ifs->removeSubscriber();
mManager->forgetSubscriber(this);
deleteLater();
}
......
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