clean transition when files with spaces exist already

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
This commit is contained in:
Matthieu Gallien 2022-02-04 16:22:02 +01:00 committed by Matthieu Gallien (Rebase PR Action)
parent e89268bdd7
commit 8decd475e5
3 changed files with 73 additions and 39 deletions

View file

@ -52,10 +52,10 @@ bool ProcessDirectoryJob::checkForInvalidFileName(const PathTuple &path,
if (entriesIter != entries.end()) { if (entriesIter != entries.end()) {
QString errorMessage; QString errorMessage;
const auto newFileNameEntry = entriesIter->second; const auto newFileNameEntry = entriesIter->second;
if (newFileNameEntry.serverEntry.isValid()) { if (entry.serverEntry.isValid() && newFileNameEntry.serverEntry.isValid()) {
errorMessage = tr("File contains trailing spaces and could not be renamed, because a file with the same name already exists on the server."); errorMessage = tr("File contains trailing spaces and could not be renamed, because a file with the same name already exists on the server.");
} }
if (newFileNameEntry.localEntry.isValid()) { if (entry.localEntry.isValid() && newFileNameEntry.localEntry.isValid()) {
errorMessage = tr("File contains trailing spaces and could not be renamed, because a file with the same name already exists locally."); errorMessage = tr("File contains trailing spaces and could not be renamed, because a file with the same name already exists locally.");
} }
@ -71,7 +71,7 @@ bool ProcessDirectoryJob::checkForInvalidFileName(const PathTuple &path,
item->_instruction = CSYNC_INSTRUCTION_ERROR; item->_instruction = CSYNC_INSTRUCTION_ERROR;
item->_status = SyncFileItem::NormalError; item->_status = SyncFileItem::NormalError;
item->_errorString = errorMessage; item->_errorString = errorMessage;
emit _discoveryData->itemDiscovered(item); processFileFinalize(item, path, false, ParentNotChanged, ParentNotChanged);
return false; return false;
} }
} }
@ -852,6 +852,39 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
processFileFinalize(item, path, recurse, recurseQueryLocal, recurseQueryServer); processFileFinalize(item, path, recurse, recurseQueryLocal, recurseQueryServer);
}; };
auto handleInvalidSpaceRename = [&] (SyncFileItem::Direction direction) {
if (_dirItem) {
path._target = _dirItem->_file + "/" + localEntry.renameName;
} else {
path._target = localEntry.renameName;
}
OCC::SyncJournalFileRecord base;
if (!_discoveryData->_statedb->getFileRecordByInode(localEntry.inode, &base)) {
dbError();
return;
}
const auto originalPath = base.path();
const auto adjustedOriginalPath = _discoveryData->adjustRenamedPath(originalPath, SyncFileItem::Down);
_discoveryData->_renamedItemsLocal.insert(originalPath, path._target);
item->_renameTarget = path._target;
path._server = adjustedOriginalPath;
if (_dirItem) {
item->_file = _dirItem->_file + "/" + localEntry.name;
} else {
item->_file = localEntry.name;
}
path._original = originalPath;
item->_originalFile = path._original;
item->_modtime = base._modtime;
item->_inode = base._inode;
item->_instruction = CSYNC_INSTRUCTION_RENAME;
item->_direction = direction;
item->_fileId = base._fileId;
item->_remotePerm = base._remotePerm;
item->_etag = base._etag;
item->_type = base._type;
};
if (!localEntry.isValid()) { if (!localEntry.isValid()) {
if (_queryLocal == ParentNotChanged && dbEntry.isValid()) { if (_queryLocal == ParentNotChanged && dbEntry.isValid()) {
// Not modified locally (ParentNotChanged) // Not modified locally (ParentNotChanged)
@ -933,6 +966,8 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
|| _discoveryData->_syncOptions._vfs->needsMetadataUpdate(*item))) { || _discoveryData->_syncOptions._vfs->needsMetadataUpdate(*item))) {
item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
item->_direction = SyncFileItem::Down; item->_direction = SyncFileItem::Down;
} else if (!localEntry.renameName.isEmpty()) {
handleInvalidSpaceRename(SyncFileItem::Up);
} }
} else if (!typeChange && isVfsWithSuffix() } else if (!typeChange && isVfsWithSuffix()
&& dbEntry.isVirtualFile() && !localEntry.isVirtualFile && dbEntry.isVirtualFile() && !localEntry.isVirtualFile
@ -1019,37 +1054,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
} }
if (!localEntry.renameName.isEmpty()) { if (!localEntry.renameName.isEmpty()) {
if (_dirItem) { handleInvalidSpaceRename(SyncFileItem::Down);
path._target = _dirItem->_file + "/" + localEntry.renameName;
} else {
path._target = localEntry.renameName;
}
OCC::SyncJournalFileRecord base;
if (!_discoveryData->_statedb->getFileRecordByInode(localEntry.inode, &base)) {
dbError();
return;
}
const auto originalPath = base.path();
const auto adjustedOriginalPath = _discoveryData->adjustRenamedPath(originalPath, SyncFileItem::Down);
_discoveryData->_renamedItemsLocal.insert(originalPath, path._target);
item->_renameTarget = path._target;
path._server = adjustedOriginalPath;
if (_dirItem) {
item->_file = _dirItem->_file + "/" + localEntry.name;
} else {
item->_file = localEntry.name;
}
path._original = originalPath;
item->_originalFile = path._original;
item->_modtime = base._modtime;
item->_inode = base._inode;
item->_instruction = CSYNC_INSTRUCTION_RENAME;
item->_direction = SyncFileItem::Down;
item->_fileId = base._fileId;
item->_remotePerm = base._remotePerm;
item->_etag = base._etag;
item->_type = base._type;
finalize(); finalize();
return; return;
} }

View file

@ -238,10 +238,14 @@ void PropagateRemoteMove::finalize()
auto &vfs = propagator()->syncOptions()._vfs; auto &vfs = propagator()->syncOptions()._vfs;
auto pinState = vfs->pinState(_item->_originalFile); auto pinState = vfs->pinState(_item->_originalFile);
// Delete old db data. const auto targetFile = propagator()->fullLocalPath(_item->_renameTarget);
propagator()->_journal->deleteFileRecord(_item->_originalFile);
if (!vfs->setPinState(_item->_originalFile, PinState::Inherited)) { if (QFileInfo::exists(targetFile)) {
qCWarning(lcPropagateRemoteMove) << "Could not set pin state of" << _item->_originalFile << "to inherited"; // Delete old db data.
propagator()->_journal->deleteFileRecord(_item->_originalFile);
if (!vfs->setPinState(_item->_originalFile, PinState::Inherited)) {
qCWarning(lcPropagateRemoteMove) << "Could not set pin state of" << _item->_originalFile << "to inherited";
}
} }
SyncFileItem newItem(*_item); SyncFileItem newItem(*_item);
@ -256,7 +260,6 @@ void PropagateRemoteMove::finalize()
} }
} }
const auto targetFile = propagator()->fullLocalPath(_item->_renameTarget);
if (!QFileInfo::exists(targetFile)) { if (!QFileInfo::exists(targetFile)) {
propagator()->_journal->commit("Remote Rename"); propagator()->_journal->commit("Remote Rename");
done(SyncFileItem::Success); done(SyncFileItem::Success);

View file

@ -360,6 +360,32 @@ private slots:
QVERIFY(fakeFolder.currentLocalState().find(fileWithSpaces)); QVERIFY(fakeFolder.currentLocalState().find(fileWithSpaces));
QVERIFY(fakeFolder.currentLocalState().find(fileTrimmed)); QVERIFY(fakeFolder.currentLocalState().find(fileTrimmed));
} }
void testCreateFileWithTrailingSpaces_localAndRemoteTrimmedExists_renameFile()
{
FakeFolder fakeFolder{FileInfo{}};
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
const QString fileWithSpaces1(" foo");
const QString fileWithSpaces2(" bar ");
const QString fileWithSpaces3("bla ");
fakeFolder.localModifier().insert(fileWithSpaces1);
fakeFolder.localModifier().insert(fileWithSpaces2);
fakeFolder.localModifier().insert(fileWithSpaces3);
fakeFolder.remoteModifier().insert(fileWithSpaces1);
fakeFolder.remoteModifier().insert(fileWithSpaces2);
fakeFolder.remoteModifier().insert(fileWithSpaces3);
QVERIFY(fakeFolder.syncOnce());
QVERIFY(fakeFolder.syncOnce());
QVERIFY(fakeFolder.syncOnce());
auto expectedState = fakeFolder.currentLocalState();
qDebug() << expectedState;
QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
}
}; };
QTEST_GUILESS_MAIN(TestLocalDiscovery) QTEST_GUILESS_MAIN(TestLocalDiscovery)