aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Vrátil <dvratil@kde.org>2016-09-08 13:51:08 (GMT)
committerDaniel Vrátil <dvratil@kde.org>2016-09-08 13:51:08 (GMT)
commit87633ed7bb8db3b22eaed5a372091fc2793ce0aa (patch)
tree602e2715c427524e86e3354d3fc59eb987efff4e
parentec825e98bd694c6b7f11d163da73477ebc27e8a1 (diff)
Fix part version handling
Make sure that part version is updated for attributes, not just payloads and that the version is really bumped. we no longer rely on client sending us the correct version number.
-rw-r--r--autotests/private/externalpartstoragetest.cpp48
-rw-r--r--autotests/server/partstreamertest.cpp16
-rw-r--r--src/core/jobs/itemmodifyjob.cpp2
-rw-r--r--src/private/externalpartstorage.cpp26
-rw-r--r--src/private/externalpartstorage_p.h6
-rw-r--r--src/server/storage/parthelper.cpp6
-rw-r--r--src/server/storage/partstreamer.cpp10
-rw-r--r--src/server/storagejanitor.cpp2
8 files changed, 42 insertions, 74 deletions
diff --git a/autotests/private/externalpartstoragetest.cpp b/autotests/private/externalpartstoragetest.cpp
index 9025695..8b6dba1 100644
--- a/autotests/private/externalpartstoragetest.cpp
+++ b/autotests/private/externalpartstoragetest.cpp
@@ -35,9 +35,6 @@ private Q_SLOTS:
void testResolveAbsolutePath_data();
void testResolveAbsolutePath();
- void testUpdateFileNameRevision_data();
- void testUpdateFileNameRevision();
-
void testNameForPartId_data();
void testNameForPartId();
@@ -108,43 +105,25 @@ void ExternalPartStorageTest::testResolveAbsolutePath()
QVERIFY(QFile::remove(path));
}
-void ExternalPartStorageTest::testUpdateFileNameRevision_data()
-{
- QTest::addColumn<QByteArray>("name");
- QTest::addColumn<QByteArray>("expectedName");
-
- QTest::newRow("no revision") << QByteArray("1234") << QByteArray("1234_r0");
- QTest::newRow("r0") << QByteArray("1234_r0") << QByteArray("1234_r1");
- QTest::newRow("r12") << QByteArray("1234_r12") << QByteArray("1234_r13");
- QTest::newRow("r123456") << QByteArray("1234_r123456") << QByteArray("1234_r123457");
-}
-
-void ExternalPartStorageTest::testUpdateFileNameRevision()
-{
- QFETCH(QByteArray, name);
- QFETCH(QByteArray, expectedName);
-
- const QByteArray newName = ExternalPartStorage::updateFileNameRevision(name);
- QCOMPARE(newName, expectedName);
-}
-
void ExternalPartStorageTest::testNameForPartId_data()
{
QTest::addColumn<qint64>("id");
+ QTest::addColumn<int>("version");
QTest::addColumn<QByteArray>("expectedName");
- QTest::newRow("0") << 0ll << QByteArray("0_r0");
- QTest::newRow("12") << 12ll << QByteArray("12_r0");
- QTest::newRow("9876543") << 9876543ll << QByteArray("9876543_r0");
+ QTest::newRow("0") << 0ll << 0 << QByteArray("0_r0");
+ QTest::newRow("12") << 12ll << 10 << QByteArray("12_r10");
+ QTest::newRow("9876543") << 9876543ll << 243243 << QByteArray("9876543_r243243");
}
void ExternalPartStorageTest::testNameForPartId()
{
QFETCH(qint64, id);
+ QFETCH(int, version);
QFETCH(QByteArray, expectedName);
- const QByteArray name = ExternalPartStorage::nameForPartId(id);
+ const QByteArray name = ExternalPartStorage::nameForPartId(id, version);
QCOMPARE(name, expectedName);
}
@@ -168,9 +147,8 @@ void ExternalPartStorageTest::testPartUpdate()
const QString filePath = ExternalPartStorage::resolveAbsolutePath(filename);
QVERIFY(QFile::exists(filePath));
- QByteArray newfilename;
- QVERIFY(ExternalPartStorage::self()->updatePartFile("newdata", filename, newfilename));
- QCOMPARE(ExternalPartStorage::updateFileNameRevision(filename), newfilename);
+ QVERIFY(ExternalPartStorage::self()->updatePartFile("newdata", filename, 10, 1));
+ const QByteArray newfilename = ExternalPartStorage::nameForPartId(10, 1);
const QString newFilePath = ExternalPartStorage::resolveAbsolutePath(newfilename);
QVERIFY(!QFile::exists(filePath));
QVERIFY(QFile::exists(newFilePath));
@@ -217,9 +195,8 @@ void ExternalPartStorageTest::testPartUpdateTrxRollback()
ExternalPartStorageTransaction trx;
- QByteArray newfilename;
- QVERIFY(ExternalPartStorage::self()->updatePartFile("newdata", filename, newfilename));
- QCOMPARE(ExternalPartStorage::updateFileNameRevision(filename), newfilename);
+ QVERIFY(ExternalPartStorage::self()->updatePartFile("newdata", filename, 10, 1));
+ const QByteArray newfilename = ExternalPartStorage::nameForPartId(10, 1);
const QString newFilePath = ExternalPartStorage::resolveAbsolutePath(newfilename);
QVERIFY(QFile::exists(filePath));
QVERIFY(QFile::exists(newFilePath));
@@ -282,9 +259,8 @@ void ExternalPartStorageTest::testPartUpdateTrxCommit()
ExternalPartStorageTransaction trx;
- QByteArray newfilename;
- QVERIFY(ExternalPartStorage::self()->updatePartFile("newdata", filename, newfilename));
- QCOMPARE(ExternalPartStorage::updateFileNameRevision(filename), newfilename);
+ QVERIFY(ExternalPartStorage::self()->updatePartFile("newdata", filename, 10, 1));
+ const QByteArray newfilename = ExternalPartStorage::nameForPartId(10, 1);
const QString newFilePath = ExternalPartStorage::resolveAbsolutePath(newfilename);
QVERIFY(QFile::exists(filePath));
QVERIFY(QFile::exists(newFilePath));
diff --git a/autotests/server/partstreamertest.cpp b/autotests/server/partstreamertest.cpp
index 3e4e56c..404ba49 100644
--- a/autotests/server/partstreamertest.cpp
+++ b/autotests/server/partstreamertest.cpp
@@ -119,11 +119,11 @@ private Q_SLOTS:
<< TestScenario::create(5, TestScenario::ClientCmd, createCommand(item))
<< TestScenario::create(5, TestScenario::ServerCmd, Protocol::StreamPayloadCommand("PLD:DATA", Protocol::StreamPayloadCommand::MetaData))
<< TestScenario::create(5, TestScenario::ClientCmd, Protocol::StreamPayloadResponse("PLD:DATA", Protocol::PartMetaData("PLD:DATA", 9)))
- << TestScenario::create(5, TestScenario::ServerCmd, Protocol::StreamPayloadCommand("PLD:DATA", Protocol::StreamPayloadCommand::Data, QString::fromLatin1("%1_r0").arg(partId)))
+ << TestScenario::create(5, TestScenario::ServerCmd, Protocol::StreamPayloadCommand("PLD:DATA", Protocol::StreamPayloadCommand::Data, QString::fromLatin1("%1_r1").arg(partId)))
<< TestScenario::create(5, TestScenario::ClientCmd, Protocol::StreamPayloadResponse("PLD:DATA"))
<< TestScenario::create(5, TestScenario::ServerCmd, Protocol::ModifyItemsResponse(item.id(), 2));
- QTest::newRow("item 1, change to external") << scenarios << QByteArray("PLD:DATA") << QByteArray("15_r0") << QByteArray("123456789") << 9ll << true << true << item;
+ QTest::newRow("item 1, change to external") << scenarios << QByteArray("PLD:DATA") << QByteArray("15_r1") << QByteArray("123456789") << 9ll << true << true << item;
}
{
@@ -132,11 +132,11 @@ private Q_SLOTS:
<< TestScenario::create(5, TestScenario::ClientCmd, createCommand(item))
<< TestScenario::create(5, TestScenario::ServerCmd, Protocol::StreamPayloadCommand("PLD:DATA", Protocol::StreamPayloadCommand::MetaData))
<< TestScenario::create(5, TestScenario::ClientCmd, Protocol::StreamPayloadResponse("PLD:DATA", Protocol::PartMetaData("PLD:DATA", 9)))
- << TestScenario::create(5, TestScenario::ServerCmd, Protocol::StreamPayloadCommand("PLD:DATA", Protocol::StreamPayloadCommand::Data, QString::fromLatin1("%1_r1").arg(partId)))
+ << TestScenario::create(5, TestScenario::ServerCmd, Protocol::StreamPayloadCommand("PLD:DATA", Protocol::StreamPayloadCommand::Data, QString::fromLatin1("%1_r2").arg(partId)))
<< TestScenario::create(5, TestScenario::ClientCmd, Protocol::StreamPayloadResponse("PLD:DATA"))
<< TestScenario::create(5, TestScenario::ServerCmd, Protocol::ModifyItemsResponse(item.id(), 3));
- QTest::newRow("item 1, update external") << scenarios << QByteArray("PLD:DATA") << QByteArray("15_r1") << QByteArray("987654321") << 9ll << true << true << item;
+ QTest::newRow("item 1, update external") << scenarios << QByteArray("PLD:DATA") << QByteArray("15_r2") << QByteArray("987654321") << 9ll << true << true << item;
}
{
@@ -145,11 +145,11 @@ private Q_SLOTS:
<< TestScenario::create(5, TestScenario::ClientCmd, createCommand(item))
<< TestScenario::create(5, TestScenario::ServerCmd, Protocol::StreamPayloadCommand("PLD:DATA", Protocol::StreamPayloadCommand::MetaData))
<< TestScenario::create(5, TestScenario::ClientCmd, Protocol::StreamPayloadResponse("PLD:DATA", Protocol::PartMetaData("PLD:DATA", 9)))
- << TestScenario::create(5, TestScenario::ServerCmd, Protocol::StreamPayloadCommand("PLD:DATA", Protocol::StreamPayloadCommand::Data, QString::fromLatin1("%1_r2").arg(partId)))
+ << TestScenario::create(5, TestScenario::ServerCmd, Protocol::StreamPayloadCommand("PLD:DATA", Protocol::StreamPayloadCommand::Data, QString::fromLatin1("%1_r3").arg(partId)))
<< TestScenario::create(5, TestScenario::ClientCmd, Protocol::StreamPayloadResponse("PLD:DATA"))
<< TestScenario::create(5, TestScenario::ServerCmd, Protocol::ModifyItemsResponse(item.id(), 4));
- QTest::newRow("item 1, external, no change") << scenarios << QByteArray("PLD:DATA") << QByteArray("15_r2") << QByteArray("987654321") << 9ll << false << true << item;
+ QTest::newRow("item 1, external, no change") << scenarios << QByteArray("PLD:DATA") << QByteArray("15_r3") << QByteArray("987654321") << 9ll << false << true << item;
}
{
@@ -221,7 +221,7 @@ private Q_SLOTS:
// Make sure no previous versions are left behind in file_db_data
for (int i = 0; i < part.version(); ++i) {
- const QByteArray fileName = QByteArray::number(part.id()) + "_r" + QByteArray::number(part.version());
+ const QByteArray fileName = QByteArray::number(part.id()) + "_r" + QByteArray::number(i);
const QString filePath = ExternalPartStorage::resolveAbsolutePath(fileName);
QVERIFY(!QFile::exists(filePath));
}
@@ -230,7 +230,7 @@ private Q_SLOTS:
// Make sure nothing is left behind in file_db_data
for (int i = 0; i <= part.version(); ++i) {
- const QByteArray fileName = QByteArray::number(part.id()) + "_r" + QByteArray::number(part.version());
+ const QByteArray fileName = QByteArray::number(part.id()) + "_r" + QByteArray::number(i);
const QString filePath = ExternalPartStorage::resolveAbsolutePath(fileName);
QVERIFY(!QFile::exists(filePath));
}
diff --git a/src/core/jobs/itemmodifyjob.cpp b/src/core/jobs/itemmodifyjob.cpp
index 0127239..155a27c 100644
--- a/src/core/jobs/itemmodifyjob.cpp
+++ b/src/core/jobs/itemmodifyjob.cpp
@@ -58,7 +58,7 @@ Protocol::PartMetaData ItemModifyJobPrivate::preparePart(const QByteArray &partN
}
mPendingData.clear();
- int version = 0;
+ int version = -1;
ItemSerializer::serialize(mItems.first(), partLabel, mPendingData, version);
return Protocol::PartMetaData(partName, mPendingData.size(), version);
diff --git a/src/private/externalpartstorage.cpp b/src/private/externalpartstorage.cpp
index 4c2cbeb..42ee555 100644
--- a/src/private/externalpartstorage.cpp
+++ b/src/private/externalpartstorage.cpp
@@ -136,7 +136,7 @@ bool ExternalPartStorage::createPartFile(const QByteArray &data, qint64 partId,
QByteArray &partFileName)
{
bool exists = false;
- partFileName = updateFileNameRevision(QByteArray::number(partId));
+ partFileName = nameForPartId(partId, 0);
const QString path = resolveAbsolutePath(partFileName, &exists);
if (exists) {
qCWarning(AKONADIPRIVATE_LOG) << "Error: asked to create a part" << partFileName << ", which already exists!";
@@ -162,7 +162,8 @@ bool ExternalPartStorage::createPartFile(const QByteArray &data, qint64 partId,
}
bool ExternalPartStorage::updatePartFile(const QByteArray &newData, const QByteArray &partFile,
- QByteArray &newPartFile)
+ qint64 partId, int version)
+
{
bool exists = false;
const QString currentPartPath = resolveAbsolutePath(partFile, &exists);
@@ -171,7 +172,7 @@ bool ExternalPartStorage::updatePartFile(const QByteArray &newData, const QByteA
return false;
}
- newPartFile = updateFileNameRevision(partFile);
+ QByteArray newPartFile = nameForPartId(partId, version);
exists = false;
const QString newPartPath = resolveAbsolutePath(newPartFile, &exists);
if (exists) {
@@ -219,23 +220,12 @@ bool ExternalPartStorage::removePartFile(const QString &partFile)
return true;
}
-QByteArray ExternalPartStorage::updateFileNameRevision(const QByteArray &filename)
+QByteArray ExternalPartStorage::nameForPartId(qint64 partId, int revision)
{
- const int revIndex = filename.indexOf("_r");
- if (revIndex > -1) {
- QByteArray rev = filename.mid(revIndex + 2);
- int r = rev.toInt();
- r++;
- rev = QByteArray::number(r);
- return filename.left(revIndex + 2) + rev;
+ if (revision < 0) {
+ revision = 0;
}
-
- return filename + "_r0";
-}
-
-QByteArray ExternalPartStorage::nameForPartId(qint64 partId)
-{
- return QByteArray::number(partId) + "_r0";
+ return QByteArray::number(partId) + "_r" + QByteArray::number(revision);
}
bool ExternalPartStorage::beginTransaction()
diff --git a/src/private/externalpartstorage_p.h b/src/private/externalpartstorage_p.h
index 6c292bf..7628732 100644
--- a/src/private/externalpartstorage_p.h
+++ b/src/private/externalpartstorage_p.h
@@ -58,11 +58,11 @@ public:
static QString resolveAbsolutePath(const QByteArray &filename, bool *exists = Q_NULLPTR, bool legacyFallback = true);
static QString resolveAbsolutePath(const QString &filename, bool *exists = Q_NULLPTR, bool legacyFallback = true);
- static QByteArray updateFileNameRevision(const QByteArray &filename);
- static QByteArray nameForPartId(qint64 partId);
+ static QByteArray nameForPartId(qint64 partId, int revision);
static QString akonadiStoragePath();
- bool updatePartFile(const QByteArray &newData, const QByteArray &partFile, QByteArray &newPartFile);
+ bool updatePartFile(const QByteArray &newData, const QByteArray &partFile,
+ qint64 partId, int version);
bool createPartFile(const QByteArray &newData, qint64 partId, QByteArray &partFileName);
bool removePartFile(const QString &partFile);
diff --git a/src/server/storage/parthelper.cpp b/src/server/storage/parthelper.cpp
index 431b1ac..4005e21 100644
--- a/src/server/storage/parthelper.cpp
+++ b/src/server/storage/parthelper.cpp
@@ -42,13 +42,17 @@ void PartHelper::update(Part *part, const QByteArray &data, qint64 dataSize)
throw PartHelperException("Invalid part");
}
+ Q_ASSERT(part->isValid());
+ part->setVersion(part->version() + 1);
+
const bool storeExternal = dataSize > DbConfig::configuredDatabase()->sizeThreshold();
QByteArray newFile;
if (part->external() && storeExternal) {
- if (!ExternalPartStorage::self()->updatePartFile(data, part->data(), newFile)) {
+ if (!ExternalPartStorage::self()->updatePartFile(data, part->data(), part->id(), part->version())) {
throw PartHelperException(QStringLiteral("Failed to update external payload part"));
}
+ newFile = ExternalPartStorage::nameForPartId(part->id(), part->version());
part->setData(newFile);
} if (!part->external() && storeExternal) {
if (!ExternalPartStorage::self()->createPartFile(data, part->id(), newFile)) {
diff --git a/src/server/storage/partstreamer.cpp b/src/server/storage/partstreamer.cpp
index e7bacc7..f1eaf98 100644
--- a/src/server/storage/partstreamer.cpp
+++ b/src/server/storage/partstreamer.cpp
@@ -81,7 +81,6 @@ bool PartStreamer::streamPayload(Part &part, const QByteArray &partName)
mError = QStringLiteral("Part name is empty");
return false;
}
- part.setVersion(metaPart.version());
if (part.datasize() != metaPart.size()) {
part.setDatasize(metaPart.size());
@@ -123,6 +122,7 @@ bool PartStreamer::streamPayloadData(Part &part, const Protocol::PartMetaData &m
}
if (part.isValid()) {
+ part.setVersion(part.version() + 1);
if (!mDataChanged) {
mDataChanged = mDataChanged || (newData != part.data());
}
@@ -153,11 +153,9 @@ bool PartStreamer::streamPayloadToFile(Part &part, const Protocol::PartMetaData
filename = part.data();
ExternalPartStorage::self()->removePartFile(
ExternalPartStorage::resolveAbsolutePath(filename));
- filename = ExternalPartStorage::updateFileNameRevision(filename);
- } else {
- // Part wasn't external, but is now
- filename = ExternalPartStorage::nameForPartId(part.id());
}
+ part.setVersion(part.version() + 1);
+ filename = ExternalPartStorage::nameForPartId(part.id(), part.version());
QFileInfo finfo(QString::fromUtf8(filename));
if (finfo.isAbsolute()) {
@@ -180,7 +178,7 @@ bool PartStreamer::streamPayloadToFile(Part &part, const Protocol::PartMetaData
return false;
}
- filename = ExternalPartStorage::nameForPartId(part.id());
+ filename = ExternalPartStorage::nameForPartId(part.id(), 0);
part.setData(filename);
if (!part.update()) {
mError = QStringLiteral("Failed to update part in database");
diff --git a/src/server/storagejanitor.cpp b/src/server/storagejanitor.cpp
index 797d340..3f80825 100644
--- a/src/server/storagejanitor.cpp
+++ b/src/server/storagejanitor.cpp
@@ -583,7 +583,7 @@ void StorageJanitor::checkSizeTreshold()
while (query.next()) {
Transaction transaction(DataStore::self());
Part part = Part::retrieveById(query.value(0).toLongLong());
- const QByteArray name = ExternalPartStorage::nameForPartId(part.id());
+ const QByteArray name = ExternalPartStorage::nameForPartId(part.id(), part.version());
const QString partPath = ExternalPartStorage::resolveAbsolutePath(name);
QFile f(partPath);
if (f.exists()) {