From fbadadc377fc8a34b1fd4086ae60fb91dcfd54d4 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 12 Jun 2014 13:45:25 +0200 Subject: [PATCH] propagator: Fix folder duplication if the folder is renamed on the server while uploading While uploading a new folder, if the folder is renamed on the server when still uploading, the result will be that the files that are already uploaded will end up in the new filder name, but the file that were not still are in the old folder. After renaming, all the new uploads wil fail with an error on this sync because the parent directory don't exist. But they were uploaded with the old name in the next sync because the renaming was not detected because the file id was not in the DB Fix the problem by fetching the file id always when creating a new directory, on the next sync, and saving it in the database ummediatly https://github.com/owncloud/enterprise/issues/191 --- csync/tests/ownCloud/t2.pl | 24 ++++++++++++++++++- src/mirall/owncloudpropagator.cpp | 6 +++++ src/mirall/owncloudpropagator.h | 1 + src/mirall/propagatorjobs.cpp | 39 +++++++++++++++++++++++++++++++ src/mirall/propagatorjobs.h | 3 +++ 5 files changed, 72 insertions(+), 1 deletion(-) diff --git a/csync/tests/ownCloud/t2.pl b/csync/tests/ownCloud/t2.pl index 5260f77064..1893f021fc 100755 --- a/csync/tests/ownCloud/t2.pl +++ b/csync/tests/ownCloud/t2.pl @@ -185,12 +185,34 @@ assertLocalAndRemoteDir( 'remoteToLocal1', 1); printInfo("Move a file from the server"); $inode = getInode('remoteToLocal1/rtl2/kb1_local_gone.jpg'); moveRemoteFile( 'remoteToLocal1/rtl2/kb1_local_gone.jpg', 'remoteToLocal1/rtl2/kb1_local_gone2.jpg'); + +#also create a new directory localy for the next test +mkdir( localDir().'superNewDir' ); +createLocalFile(localDir(). 'superNewDir/f1', 1234 ); +createLocalFile(localDir(). 'superNewDir/f2', 1324 ); +my $superNewDirInode = getInode('superNewDir'); + + csync(); -assertLocalAndRemoteDir( 'remoteToLocal1', 1); +assertLocalAndRemoteDir( '', 1); $inode2 = getInode('remoteToLocal1/rtl2/kb1_local_gone2.jpg'); assert( $inode == $inode2, "Inode has changed 3!"); +printInfo("Move a newly created directory"); +moveRemoteFile('superNewDir', 'superNewDirRenamed'); +#also add new files in new directory +createLocalFile(localDir(). 'superNewDir/f3' , 2456 ); +$inode = getInode('superNewDir/f3'); + +csync(); +assertLocalAndRemoteDir( '', 1); +assert( ! -e localDir().'superNewDir' ); + +$inode2 = getInode('superNewDir/f3'); +assert( $inode == $inode2, "Inode of f3 changed"); +$inode2 = getInode('superNewDir'); +assert( $superNewDirInode == $inode2, "Inode of superNewDir changed"); cleanup(); diff --git a/src/mirall/owncloudpropagator.cpp b/src/mirall/owncloudpropagator.cpp index f769277e45..d5ff55f2b7 100644 --- a/src/mirall/owncloudpropagator.cpp +++ b/src/mirall/owncloudpropagator.cpp @@ -427,6 +427,12 @@ void PropagateDirectory::slotSubJobReady() } if (_item._should_update_etag && _item._instruction != CSYNC_INSTRUCTION_REMOVE) { + if (PropagateRemoteMkdir* mkdir = qobject_cast(_firstJob.data())) { + // special case from MKDIR, get the fileId from the job there + if (_item._fileId.isEmpty() && !mkdir->_item._fileId.isEmpty()) { + _item._fileId = mkdir->_item._fileId; + } + } SyncJournalFileRecord record(_item, _propagator->_localDir + _item._file); _propagator->_journal->setFileRecord(record); } diff --git a/src/mirall/owncloudpropagator.h b/src/mirall/owncloudpropagator.h index ab99ef2dcc..d63ec08f39 100644 --- a/src/mirall/owncloudpropagator.h +++ b/src/mirall/owncloudpropagator.h @@ -25,6 +25,7 @@ struct hbf_transfer_s; struct ne_session_s; struct ne_decompress_s; +typedef struct ne_prop_result_set_s ne_prop_result_set; namespace Mirall { diff --git a/src/mirall/propagatorjobs.cpp b/src/mirall/propagatorjobs.cpp index 087d24d549..081559f43f 100644 --- a/src/mirall/propagatorjobs.cpp +++ b/src/mirall/propagatorjobs.cpp @@ -154,6 +154,35 @@ void PropagateRemoteRemove::start() done(SyncFileItem::Success); } +/* The list of properties that is fetched in PropFind after a MKCOL */ +static const ne_propname ls_props[] = { + { "DAV:", "getetag"}, + { "http://owncloud.org/ns", "id"}, + { NULL, NULL } +}; + +/* + * Parse the PROPFIND result after a MKCOL + */ +void PropagateRemoteMkdir::propfind_results(void *userdata, + const ne_uri *uri, + const ne_prop_result_set *set) +{ + PropagateRemoteMkdir *job = static_cast(userdata); + + job->_item._etag = parseEtag(ne_propset_value( set, &ls_props[0] )); + + const char* fileId = ne_propset_value( set, &ls_props[1] ); + if (fileId) { + job->_item._fileId = fileId; + qDebug() << "MKCOL: " << uri << " FileID set it to " << fileId; + + // save the file id already so we can detect rename + SyncJournalFileRecord record(job->_item, job->_propagator->_localDir + job->_item._renameTarget); + job->_propagator->_journal->setFileRecord(record); + } +} + void PropagateRemoteMkdir::start() { if (_propagator->_abortRequested.fetchAndAddRelaxed(0)) @@ -174,6 +203,16 @@ void PropagateRemoteMkdir::start() return; } + // Get the fileid + // This is required so that wa can detect moves even if the folder is renamed on the server + // while files are still uploading + // TODO: Now we have to do a propfind because the server does not give the file id in the request + // https://github.com/owncloud/core/issues/9000 + + ne_propfind_handler *hdl = ne_propfind_create(_propagator->_session, uri.data(), 0); + ne_propfind_named(hdl, ls_props, propfind_results, this); + ne_propfind_destroy(hdl); + done(SyncFileItem::Success); } diff --git a/src/mirall/propagatorjobs.h b/src/mirall/propagatorjobs.h index 8a47f2a52c..3fc512acfe 100644 --- a/src/mirall/propagatorjobs.h +++ b/src/mirall/propagatorjobs.h @@ -94,6 +94,9 @@ class PropagateRemoteMkdir : public PropagateNeonJob { public: PropagateRemoteMkdir (OwncloudPropagator* propagator,const SyncFileItem& item) : PropagateNeonJob(propagator, item) {} void start(); +private: + static void propfind_results(void *userdata, const ne_uri *uri, const ne_prop_result_set *set); + friend class PropagateDirectory; // So it can access the _item; }; class PropagateLocalRename : public PropagateItemJob { Q_OBJECT