Verified Commit 8e61c1c4 authored by Daniel Vrátil's avatar Daniel Vrátil 🤖

Server: fix fetching of attributes with empty data

There's a special path in ItemFetchHelper that skips an Item if the requested
part has an empty body and IgnoreError flag is set to true. This can of course
happen for payload parts, which can be expired, but is also perfectly valid
situation for attributes, which can simply have empty body by default, since
they never expire.

The whole logic there needs some revisiting, but that's for another patch and
task, since it has potential to break code that relies on all the
weirdness in the retrieval code.
parent e467dcd5
......@@ -18,7 +18,6 @@
*/
#include "itemfetchtest.h"
#include "collectionpathresolver.h"
#include "testattribute.h"
#include <attributefactory.h>
......@@ -43,18 +42,16 @@ void ItemFetchTest::initTestCase()
void ItemFetchTest::testFetch()
{
CollectionPathResolver *resolver = new CollectionPathResolver(QStringLiteral("res1"), this);
AKVERIFYEXEC(resolver);
int colId = resolver->collection();
const int colId = AkonadiTest::collectionIdFromPath(QStringLiteral("res1"));
QVERIFY(colId > -1);
// listing of an empty folder
ItemFetchJob *job = new ItemFetchJob(Collection(colId), this);
AKVERIFYEXEC(job);
QVERIFY(job->items().isEmpty());
resolver = new CollectionPathResolver(QStringLiteral("res1/foo"), this);
AKVERIFYEXEC(resolver);
int colId2 = resolver->collection();
const int colId2 = AkonadiTest::collectionIdFromPath(QStringLiteral("res1/foo"));
QVERIFY(colId > -1);
// listing of a non-empty folder
job = new ItemFetchJob(Collection(colId2), this);
......@@ -186,9 +183,8 @@ void ItemFetchTest::testMultipartFetch()
QFETCH(bool, fetchSinglePayload);
QFETCH(bool, fetchSingleAttr);
CollectionPathResolver *resolver = new CollectionPathResolver(QStringLiteral("res1/foo"), this);
AKVERIFYEXEC(resolver);
int colId = resolver->collection();
int colId = AkonadiTest::collectionIdFromPath(QStringLiteral("res1/foo"));
QVERIFY(colId >= 0);
Item item;
item.setMimeType(QStringLiteral("application/octet-stream"));
......@@ -275,6 +271,49 @@ void ItemFetchTest::testAncestorRetrieval()
QCOMPARE(c2.remoteId(), QLatin1String("6"));
const Collection c3 = c2.parentCollection();
QCOMPARE(c3, Collection::root());
}
void ItemFetchTest::testRetrievalOfAttributeWithEmptyBody()
{
const auto colId = AkonadiTest::collectionIdFromPath(QStringLiteral("res1/foo"));
QVERIFY(colId > -1);
auto testFetch = new ItemFetchJob(Collection(colId), this);
AKVERIFYEXEC(testFetch);
const auto initialCount = testFetch->items().count();
Item item;
item.setMimeType(QStringLiteral("application/octet-stream"));
item.setPayload<QByteArray>("body data");
auto attr = AttributeFactory::createAttribute("EMPTY");
item.addAttribute(attr);
auto *create = new ItemCreateJob(item, Collection(colId), this);
AKVERIFYEXEC(create);
item = create->item();
// Direct fetch
auto *job = new ItemFetchJob(Item(item.id()), this);
job->fetchScope().fetchAllAttributes();
job->fetchScope().fetchFullPayload();
job->fetchScope().setFetchRemoteIdentification(true);
job->fetchScope().setIgnoreRetrievalErrors(true);
AKVERIFYEXEC(job);
QCOMPARE(job->items().count(), 1);
const auto &fetched = job->items().at(0);
QCOMPARE(fetched.id(), item.id());
QVERIFY(fetched.hasAttribute("EMPTY"));
QVERIFY(fetched.attribute("EMPTY")->serialized().isEmpty());
// Folder fetch
job = new ItemFetchJob(Collection(colId), this);
job->fetchScope().fetchAllAttributes();
job->fetchScope().fetchFullPayload();
job->fetchScope().setFetchRemoteIdentification(true);
job->fetchScope().setIgnoreRetrievalErrors(true);
AKVERIFYEXEC(job);
QCOMPARE(job->items().count(), initialCount + 1);
}
......@@ -34,6 +34,7 @@ private Q_SLOTS:
void testMultipartFetch();
void testRidFetch();
void testAncestorRetrieval();
void testRetrievalOfAttributeWithEmptyBody();
};
#endif
......@@ -602,13 +602,14 @@ bool ItemFetchHelper::fetchItems(std::function<void(Protocol::FetchItemsResponse
const QByteArray data = Utils::variantToByteArray(partQuery.value(PartQueryDataColumn));
if (mItemFetchScope.checkCachedPayloadPartsOnly()) {
if (!data.isEmpty()) {
cachedParts << ptIter.value();
cachedParts.push_back(ptIter.value());
}
partQuery.next();
} else {
if (mItemFetchScope.ignoreErrors() && data.isEmpty()) {
} else if (mItemFetchScope.fullPayload() || mItemFetchScope.allAttributes() || mItemFetchScope.requestedParts().contains(ptIter.value())) {
// FIXME: Should this fail the entire retrieval, if IgnoreErrors is not set?
if (ptIter->startsWith("PLD") && mItemFetchScope.ignoreErrors() && data.isEmpty()) {
//We wanted the payload, couldn't get it, and are ignoring errors. Skip the item.
//This is not an error though, it's fine to have empty payload parts (to denote existing but not cached parts)
//This is not an error though, it's fine to have empty payload parts (to denote existing but not cached parts).
//Empty attributes are never an error, since they never expire.
qCDebug(AKONADISERVER_LOG) << "item" << id << "has an empty payload part in parttable for part" << metaPart.name();
skipItem = true;
break;
......@@ -621,13 +622,9 @@ bool ItemFetchHelper::fetchItems(std::function<void(Protocol::FetchItemsResponse
partData.setData(data);
}
partData.setMetaData(metaPart);
if (mItemFetchScope.requestedParts().contains(ptIter.value()) || mItemFetchScope.fullPayload() || mItemFetchScope.allAttributes()) {
parts.append(partData);
}
partQuery.next();
parts.append(partData);
}
partQuery.next();
}
response.setParts(parts);
......
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