mirror of https://github.com/nextcloud/desktop
SyncEngine: Fix renaming a folder should keep the selective sync state
Issue #5224 Two problems: - In the discovery phase, we need to check the selective sync entries of the source path in case of renames. - When the rename is done, we need to actually update the black list in the database.
This commit is contained in:
parent
e0a36ab313
commit
d80d5a8ee4
|
@ -20,6 +20,7 @@
|
|||
|
||||
extern "C" {
|
||||
#include "csync_private.h"
|
||||
#include "csync_rename.h"
|
||||
}
|
||||
|
||||
#include <map>
|
||||
|
@ -93,5 +94,9 @@ char* csync_rename_adjust_path_source(CSYNC* ctx, const char* path)
|
|||
return c_strdup(path);
|
||||
}
|
||||
|
||||
bool csync_rename_count(CSYNC *ctx) {
|
||||
csync_rename_s* d = csync_rename_s::get(ctx);
|
||||
return d->folder_renamed_from.size();
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -32,6 +32,8 @@ char OCSYNC_EXPORT *csync_rename_adjust_path(CSYNC *ctx, const char *path);
|
|||
char OCSYNC_EXPORT *csync_rename_adjust_path_source(CSYNC *ctx, const char *path);
|
||||
void OCSYNC_EXPORT csync_rename_destroy(CSYNC *ctx);
|
||||
void OCSYNC_EXPORT csync_rename_record(CSYNC *ctx, const char *from, const char *to);
|
||||
/* Return the amount of renamed item recorded */
|
||||
bool OCSYNC_EXPORT csync_rename_count(CSYNC *ctx);
|
||||
|
||||
#ifdef __cplusplus
|
||||
}
|
||||
|
|
|
@ -14,6 +14,7 @@
|
|||
|
||||
#include "discoveryphase.h"
|
||||
#include <csync_private.h>
|
||||
#include <csync_rename.h>
|
||||
#include <qdebug.h>
|
||||
|
||||
#include <QUrl>
|
||||
|
@ -51,7 +52,7 @@ static bool findPathInList(const QStringList &list, const QString &path)
|
|||
return pathSlash.startsWith(*it);
|
||||
}
|
||||
|
||||
bool DiscoveryJob::isInSelectiveSyncBlackList(const QString& path) const
|
||||
bool DiscoveryJob::isInSelectiveSyncBlackList(const char *path) const
|
||||
{
|
||||
if (_selectiveSyncBlackList.isEmpty()) {
|
||||
// If there is no black list, everything is allowed
|
||||
|
@ -59,13 +60,25 @@ bool DiscoveryJob::isInSelectiveSyncBlackList(const QString& path) const
|
|||
}
|
||||
|
||||
// Block if it is in the black list
|
||||
return findPathInList(_selectiveSyncBlackList, path);
|
||||
if (findPathInList(_selectiveSyncBlackList, QString::fromUtf8(path))) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Also try to adjust the path if there was renames
|
||||
if (csync_rename_count(_csync_ctx)) {
|
||||
QScopedPointer<char, QScopedPointerPodDeleter> adjusted(
|
||||
csync_rename_adjust_path_source(_csync_ctx, path));
|
||||
if (strcmp(adjusted.data(), path) != 0) {
|
||||
return findPathInList(_selectiveSyncBlackList, QString::fromUtf8(adjusted.data()));
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
int DiscoveryJob::isInSelectiveSyncBlackListCallback(void *data, const char *path)
|
||||
{
|
||||
return static_cast<DiscoveryJob*>(data)->isInSelectiveSyncBlackList(QString::fromUtf8(path));
|
||||
return static_cast<DiscoveryJob*>(data)->isInSelectiveSyncBlackList(path);
|
||||
}
|
||||
|
||||
bool DiscoveryJob::checkSelectiveSyncNewFolder(const QString& path)
|
||||
|
|
|
@ -174,7 +174,7 @@ class DiscoveryJob : public QObject {
|
|||
* return true if the given path should be ignored,
|
||||
* false if the path should be synced
|
||||
*/
|
||||
bool isInSelectiveSyncBlackList(const QString &path) const;
|
||||
bool isInSelectiveSyncBlackList(const char* path) const;
|
||||
static int isInSelectiveSyncBlackListCallback(void *, const char *);
|
||||
bool checkSelectiveSyncNewFolder(const QString &path);
|
||||
static int checkSelectiveSyncNewFolderCallback(void*, const char*);
|
||||
|
|
|
@ -13,6 +13,7 @@
|
|||
*/
|
||||
|
||||
#include "propagateremotemove.h"
|
||||
#include "propagatorjobs.h"
|
||||
#include "owncloudpropagator_p.h"
|
||||
#include "account.h"
|
||||
#include "syncjournalfilerecord.h"
|
||||
|
@ -175,10 +176,45 @@ void PropagateRemoteMove::finalize()
|
|||
done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
|
||||
return;
|
||||
}
|
||||
|
||||
if (_item->_isDirectory) {
|
||||
if (!adjustSelectiveSync(_propagator->_journal, _item->_file, _item->_renameTarget)) {
|
||||
done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
_propagator->_journal->commit("Remote Rename");
|
||||
done(SyncFileItem::Success);
|
||||
}
|
||||
|
||||
bool PropagateRemoteMove::adjustSelectiveSync(SyncJournalDb *journal, const QString &from_, const QString &to_)
|
||||
{
|
||||
bool ok;
|
||||
// We only care about preserving the blacklist. The white list should anyway be empty.
|
||||
// And the undecided list will be repopulated on the next sync, if there is anything too big.
|
||||
QStringList list = journal->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &ok);
|
||||
if (!ok)
|
||||
return false;
|
||||
|
||||
bool changed = false;
|
||||
Q_ASSERT(!from_.endsWith(QLatin1String("/")));
|
||||
Q_ASSERT(!to_.endsWith(QLatin1String("/")));
|
||||
QString from = from_ + QLatin1String("/");
|
||||
QString to = to_ + QLatin1String("/");
|
||||
|
||||
for (auto it = list.begin(); it != list.end(); ++it) {
|
||||
if (it->startsWith(from)) {
|
||||
*it = it->replace(0, from.size(), to);
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (changed) {
|
||||
journal->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, list);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
|
|
|
@ -51,6 +51,12 @@ public:
|
|||
void start() Q_DECL_OVERRIDE;
|
||||
void abort() Q_DECL_OVERRIDE;
|
||||
JobParallelism parallelism() Q_DECL_OVERRIDE { return OCC::PropagatorJob::WaitForFinishedInParentDirectory; }
|
||||
|
||||
/**
|
||||
* Rename the directory in the selective sync list
|
||||
*/
|
||||
static bool adjustSelectiveSync(SyncJournalDb *journal, const QString &from, const QString &to);
|
||||
|
||||
private slots:
|
||||
void slotMoveJobFinished();
|
||||
void finalize();
|
||||
|
|
|
@ -15,7 +15,7 @@
|
|||
|
||||
#include "propagatorjobs.h"
|
||||
#include "owncloudpropagator_p.h"
|
||||
|
||||
#include "propagateremotemove.h"
|
||||
#include "utility.h"
|
||||
#include "syncjournaldb.h"
|
||||
#include "syncjournalfilerecord.h"
|
||||
|
@ -231,6 +231,7 @@ void PropagateLocalRename::start()
|
|||
_propagator->_journal->deleteFileRecord(_item->_originalFile);
|
||||
|
||||
// store the rename file name in the item.
|
||||
const auto oldFile = _item->_file;
|
||||
_item->_file = _item->_renameTarget;
|
||||
|
||||
SyncJournalFileRecord record(*_item, targetFile);
|
||||
|
@ -245,9 +246,14 @@ void PropagateLocalRename::start()
|
|||
done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
if (!PropagateRemoteMove::adjustSelectiveSync(_propagator->_journal, oldFile, _item->_renameTarget)) {
|
||||
done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
|
||||
return;
|
||||
}
|
||||
}
|
||||
_propagator->_journal->commit("localRename");
|
||||
|
||||
_propagator->_journal->commit("localRename");
|
||||
|
||||
done(SyncFileItem::Success);
|
||||
}
|
||||
|
|
|
@ -28,6 +28,7 @@ inline QByteArray generateFileId() {
|
|||
|
||||
class PathComponents : public QStringList {
|
||||
public:
|
||||
PathComponents(const char *path) : PathComponents{QString::fromUtf8(path)} {}
|
||||
PathComponents(const QString &path) : QStringList{path.split('/', QString::SkipEmptyParts)} { }
|
||||
PathComponents(const QStringList &pathComponents) : QStringList{pathComponents} { }
|
||||
|
||||
|
@ -90,8 +91,9 @@ public:
|
|||
void mkdir(const QString &relativePath) override {
|
||||
_rootDir.mkpath(relativePath);
|
||||
}
|
||||
void rename(const QString &, const QString &) override {
|
||||
Q_ASSERT(!"not implemented");
|
||||
void rename(const QString &from, const QString &to) override {
|
||||
QVERIFY(_rootDir.exists(from));
|
||||
QVERIFY(_rootDir.rename(from, to));
|
||||
}
|
||||
};
|
||||
|
||||
|
@ -457,6 +459,36 @@ public:
|
|||
qint64 readData(char *, qint64) override { return 0; }
|
||||
};
|
||||
|
||||
class FakeMoveReply : public QNetworkReply
|
||||
{
|
||||
Q_OBJECT
|
||||
public:
|
||||
FakeMoveReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, QObject *parent)
|
||||
: QNetworkReply{parent} {
|
||||
setRequest(request);
|
||||
setUrl(request.url());
|
||||
setOperation(op);
|
||||
open(QIODevice::ReadOnly);
|
||||
|
||||
Q_ASSERT(request.url().path().startsWith(sRootUrl.path()));
|
||||
QString fileName = request.url().path().mid(sRootUrl.path().length());
|
||||
QString destPath = request.rawHeader("Destination");
|
||||
Q_ASSERT(destPath.startsWith(sRootUrl.path()));
|
||||
QString dest = destPath.mid(sRootUrl.path().length());
|
||||
remoteRootFileInfo.rename(fileName, dest);
|
||||
QMetaObject::invokeMethod(this, "respond", Qt::QueuedConnection);
|
||||
}
|
||||
|
||||
Q_INVOKABLE void respond() {
|
||||
setAttribute(QNetworkRequest::HttpStatusCodeAttribute, 201);
|
||||
emit metaDataChanged();
|
||||
emit finished();
|
||||
}
|
||||
|
||||
void abort() override { }
|
||||
qint64 readData(char *, qint64) override { return 0; }
|
||||
};
|
||||
|
||||
class FakeGetReply : public QNetworkReply
|
||||
{
|
||||
Q_OBJECT
|
||||
|
@ -552,6 +584,8 @@ protected:
|
|||
return new FakeMkcolReply{_remoteRootFileInfo, op, request, this};
|
||||
else if (verb == QLatin1String("DELETE"))
|
||||
return new FakeDeleteReply{_remoteRootFileInfo, op, request, this};
|
||||
else if (verb == QLatin1String("MOVE"))
|
||||
return new FakeMoveReply{_remoteRootFileInfo, op, request, this};
|
||||
else {
|
||||
qDebug() << verb << outgoingData;
|
||||
Q_UNREACHABLE();
|
||||
|
|
|
@ -7,6 +7,7 @@
|
|||
|
||||
#include <QtTest>
|
||||
#include "syncenginetestutils.h"
|
||||
#include <syncengine.h>
|
||||
|
||||
using namespace OCC;
|
||||
|
||||
|
@ -137,8 +138,8 @@ private slots:
|
|||
fakeFolder.syncOnce();
|
||||
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
|
||||
auto oldState = fakeFolder.currentLocalState();
|
||||
QVERIFY(oldState.find(PathComponents("folder/folderB/folderA/file.txt")));
|
||||
QVERIFY(!oldState.find(PathComponents("folder/folderA/file.txt")));
|
||||
QVERIFY(oldState.find("folder/folderB/folderA/file.txt"));
|
||||
QVERIFY(!oldState.find("folder/folderA/file.txt"));
|
||||
|
||||
// This sync should not remove the file
|
||||
fakeFolder.syncOnce();
|
||||
|
@ -146,6 +147,72 @@ private slots:
|
|||
QCOMPARE(fakeFolder.currentLocalState(), oldState);
|
||||
|
||||
}
|
||||
|
||||
void testSelectiveSyncModevFolder() {
|
||||
// issue #5224
|
||||
FakeFolder fakeFolder{FileInfo{ QString(), {
|
||||
FileInfo { QStringLiteral("parentFolder"), {
|
||||
FileInfo{ QStringLiteral("subFolderA"), { { QStringLiteral("fileA.txt"), 400 } } },
|
||||
FileInfo{ QStringLiteral("subFolderB"), { { QStringLiteral("fileB.txt"), 400 } } }
|
||||
}
|
||||
}}}};
|
||||
|
||||
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
|
||||
auto expectedServerState = fakeFolder.currentRemoteState();
|
||||
|
||||
// Remove subFolderA with selectiveSync:
|
||||
fakeFolder.syncEngine().journal()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList,
|
||||
{"parentFolder/subFolderA/"});
|
||||
fakeFolder.syncEngine().journal()->avoidReadFromDbOnNextSync("parentFolder/subFolderA/");
|
||||
|
||||
fakeFolder.syncOnce();
|
||||
|
||||
{
|
||||
// Nothing changed on the server
|
||||
QCOMPARE(fakeFolder.currentRemoteState(), expectedServerState);
|
||||
// The local state should not have subFolderA
|
||||
auto remoteState = fakeFolder.currentRemoteState();
|
||||
remoteState.remove("parentFolder/subFolderA");
|
||||
QCOMPARE(fakeFolder.currentLocalState(), remoteState);
|
||||
}
|
||||
|
||||
// Rename parentFolder on the server
|
||||
fakeFolder.remoteModifier().rename("parentFolder", "parentFolderRenamed");
|
||||
expectedServerState = fakeFolder.currentRemoteState();
|
||||
fakeFolder.syncOnce();
|
||||
|
||||
{
|
||||
QCOMPARE(fakeFolder.currentRemoteState(), expectedServerState);
|
||||
auto remoteState = fakeFolder.currentRemoteState();
|
||||
// The subFolderA should still be there on the server.
|
||||
QVERIFY(remoteState.find("parentFolderRenamed/subFolderA/fileA.txt"));
|
||||
// But not on the client because of the selective sync
|
||||
remoteState.remove("parentFolderRenamed/subFolderA");
|
||||
QCOMPARE(fakeFolder.currentLocalState(), remoteState);
|
||||
}
|
||||
|
||||
// Rename it again, locally this time.
|
||||
fakeFolder.localModifier().rename("parentFolderRenamed", "parentThirdName");
|
||||
fakeFolder.syncOnce();
|
||||
|
||||
{
|
||||
auto remoteState = fakeFolder.currentRemoteState();
|
||||
// The subFolderA should still be there on the server.
|
||||
QVERIFY(remoteState.find("parentThirdName/subFolderA/fileA.txt"));
|
||||
// But not on the client because of the selective sync
|
||||
remoteState.remove("parentThirdName/subFolderA");
|
||||
QCOMPARE(fakeFolder.currentLocalState(), remoteState);
|
||||
|
||||
expectedServerState = fakeFolder.currentRemoteState();
|
||||
QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItem &, const PropagatorJob &)));
|
||||
fakeFolder.syncOnce(); // This sync should do nothing
|
||||
QCOMPARE(completeSpy.count(), 0);
|
||||
|
||||
QCOMPARE(fakeFolder.currentRemoteState(), expectedServerState);
|
||||
QCOMPARE(fakeFolder.currentLocalState(), remoteState);
|
||||
}
|
||||
}
|
||||
|
||||
};
|
||||
|
||||
QTEST_GUILESS_MAIN(TestSyncEngine)
|
||||
|
|
Loading…
Reference in New Issue