Permissions: When moving is not allowed, fallback to delete and upload

We decided that we never want to rename a directory behind the
back of the user as the user may be using files in the directory
during the sync.
If moving is not allowed, we just erase the inode form the database so
the next sync will try to do an upload and delete and recover from there
using normal resolution.

This also add some code to update the inode back to the db when it is detected
as changed.

csync/src/csync_reconcile.c

@@ -184,6 +184,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
                         csync_vio_set_file_id( other->file_id, cur->file_id );
                     }
                     other->inode = cur->inode;
+                    other->should_update_etag = true;
                     cur->instruction = CSYNC_INSTRUCTION_NONE;
                 } else if (other->instruction == CSYNC_INSTRUCTION_REMOVE) {
                     other->instruction = CSYNC_INSTRUCTION_RENAME;
@@ -193,6 +194,7 @@ static int _csync_merge_algorithm_visitor(void *obj, void *data) {
                         csync_vio_set_file_id( other->file_id, cur->file_id );
                     }
                     other->inode = cur->inode;
+                    other->should_update_etag = true;
                     cur->instruction = CSYNC_INSTRUCTION_NONE;
                 } else if (other->instruction == CSYNC_INSTRUCTION_NEW) {
                     CSYNC_LOG(CSYNC_LOG_PRIORITY_TRACE, "OOOO=> NEW detected in other tree!");

csync/src/csync_update.c

@@ -254,8 +254,9 @@ static int _csync_detect_update(CSYNC *ctx, const char *file,
             st->instruction = CSYNC_INSTRUCTION_EVAL;
             goto out;
         }
-        bool metadata_differ = ctx->current == REMOTE_REPLICA && (!c_streq(fs->file_id, tmp->file_id)
-                                                            || !c_streq(fs->remotePerm, tmp->remotePerm));
+        bool metadata_differ = (ctx->current == REMOTE_REPLICA && (!c_streq(fs->file_id, tmp->file_id)
+                                                            || !c_streq(fs->remotePerm, tmp->remotePerm)))
+                             || (ctx->current == LOCAL_REPLICA && fs->inode != tmp->inode);
         if (type == CSYNC_FTW_TYPE_DIR && ctx->current == REMOTE_REPLICA
                 && !metadata_differ && !ctx->read_from_db_disabled) {
             /* If both etag and file id are equal for a directory, read all contents from
@@ -691,8 +692,7 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn,
 
     if (flag == CSYNC_FTW_FLAG_DIR && ctx->current_fs
         && (ctx->current_fs->instruction == CSYNC_INSTRUCTION_EVAL ||
-            ctx->current_fs->instruction == CSYNC_INSTRUCTION_NEW ||
-            ctx->current_fs->instruction == CSYNC_INSTRUCTION_EVAL_RENAME)) {
+            ctx->current_fs->instruction == CSYNC_INSTRUCTION_NEW)) {
         ctx->current_fs->should_update_etag = true;
     }
 

csync/tests/ownCloud/t7.pl

@@ -57,9 +57,9 @@ createRemoteDir( "normalDirectory_PERM_CKDNV_" );
 glob_put( "$tmpdir/*", "normalDirectory_PERM_CKDNV_" );
 createRemoteDir( "readonlyDirectory_PERM_M_" );
 glob_put( "$tmpdir/*", "readonlyDirectory_PERM_M_" );
-createRemoteDir( "readonlyDirectory_PERM_M_/subdir_PERM_CKDNV_" );
-createRemoteDir( "readonlyDirectory_PERM_M_/subdir_PERM_CKDNV_/subsubdir_PERM_CKDNV_" );
-glob_put( "$tmpdir/normalFile_PERM_WVND_.data", "readonlyDirectory_PERM_M_/subdir_PERM_CKDNV_/subsubdir_PERM_CKDNV_" );
+createRemoteDir( "readonlyDirectory_PERM_M_/subdir_PERM_CK_" );
+createRemoteDir( "readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_" );
+glob_put( "$tmpdir/normalFile_PERM_WVND_.data", "readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_" );
 
 
 csync();
@@ -150,13 +150,74 @@ printInfo( "remove the read only directory" );
 system("rm -r " . localDir().'readonlyDirectory_PERM_M_' );
 csync();
 assert( -e localDir(). 'readonlyDirectory_PERM_M_/cannotBeRemoved_PERM_WVN_.data' );
-assert( -e localDir(). 'readonlyDirectory_PERM_M_/subdir_PERM_CKDNV_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data' );
+assert( -e localDir(). 'readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data' );
+assertLocalAndRemoteDir( '', 0);
+
+
+#######################################################################
+printInfo( "move a directory in a outside read only folder" );
+#Missing directory should be restored
+#new directory should be uploaded
+system("mv " . localDir().'readonlyDirectory_PERM_M_/subdir_PERM_CK_ ' . localDir().'normalDirectory_PERM_CKDNV_/subdir_PERM_CKDNV_'  );
+
+# two syncs may be necessary for now
+csync();
+csync();
+
+# old name restored
+assert( -e localDir(). 'readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data' );
+
+# new still exist
+assert( -e localDir(). 'normalDirectory_PERM_CKDNV_/subdir_PERM_CKDNV_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data' );
+
 assertLocalAndRemoteDir( '', 0);
 
 
 
 
 
+#######################################################################
+printInfo( "rename a directory in a read only folder and move a directory to a read-only" );
+
+# do a sync to update the database
+csync();
+
+#1. rename a directory in a read only folder
+#Missing directory should be restored
+#new directory should stay but not be uploaded
+system("mv " . localDir().'readonlyDirectory_PERM_M_/subdir_PERM_CK_ ' . localDir().'readonlyDirectory_PERM_M_/newname_PERM_CK_'  );
+
+#2. move a directory from read to read only  (move the directory from previous step)
+system("mv " . localDir().'normalDirectory_PERM_CKDNV_/subdir_PERM_CKDNV_ ' . localDir().'readonlyDirectory_PERM_M_/moved_PERM_CK_'  );
+
+# two syncs may be necessary for now
+csync();
+csync();
+
+#1.
+# old name restored
+assert( -e localDir(). 'readonlyDirectory_PERM_M_/subdir_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data' );
+
+# new still exist
+assert( -e localDir(). 'readonlyDirectory_PERM_M_/newname_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data' );
+# but is not on server: so remove for assertLocalAndRemoteDir
+system("rm -r " . localDir(). "readonlyDirectory_PERM_M_/newname_PERM_CK_");
+
+#2.
+# old removed
+assert( ! -e localDir(). 'normalDirectory_PERM_CKDNV_/subdir_PERM_CKDNV_/' );
+# new still there
+assert( -e localDir(). 'readonlyDirectory_PERM_M_/moved_PERM_CK_/subsubdir_PERM_CKDNV_/normalFile_PERM_WVND_.data' );
+#but not on server
+system("rm -r " . localDir(). "readonlyDirectory_PERM_M_/moved_PERM_CK_");
+
+assertLocalAndRemoteDir( '', 0);
+
+
+
+cleanup();
+
+
 
 
 

src/mirall/syncengine.cpp

@@ -336,7 +336,7 @@ int SyncEngine::treewalkFile( TREE_WALK_FILE *file, bool remote )
             _journal->setFileRecord(SyncJournalFileRecord(item, _localPath + item._file));
             item._should_update_etag = false;
         }
-        if (item._isDirectory && remote) {
+        if (item._isDirectory && (remote || file->should_update_etag)) {
             // Because we want still to update etags of directories
             dir = SyncFileItem::None;
         } else {
@@ -846,6 +846,8 @@ void SyncEngine::checkForPermission()
                     }
                 }
 
+#if 0 /* We don't like the idea of renaming behind user's back, as the user may be working with the files */
+
                 if (!sourceOK && !destinationOK) {
                     // Both the source and the destination won't allow move.  Move back to the original
                     std::swap(it->_file, it->_renameTarget);
@@ -853,7 +855,9 @@ void SyncEngine::checkForPermission()
                     it->_errorString = tr("Move not allowed, item restored");
                     it->_isRestoration = true;
                     qDebug() << "checkForPermission: MOVING BACK" << it->_file;
-                } else if (!sourceOK || !destinationOK) {
+                } else
+#endif
+                if (!sourceOK || !destinationOK) {
                     // One of them is not possible, just throw an error
                     it->_instruction = CSYNC_INSTRUCTION_ERROR;
                     it->_status = SyncFileItem::NormalError;
@@ -863,10 +867,17 @@ void SyncEngine::checkForPermission()
 
                     qDebug() << "checkForPermission: ERROR MOVING" << it->_file << errorString;
 
+                    // Avoid a rename on next sync:
+                    // TODO:  do the resolution now already so we don't need two sync
+                    //  At this point we would need to go back to the propagate phase on both remote to take
+                    //  the decision.
+                    _journal->avoidRenamesOnNextSync(it->_file);
+
+
                     if (it->_isDirectory) {
-                        const QString path = it->_file + QLatin1Char('/');
+                        const QString path = it->_renameTarget + QLatin1Char('/');
                         for (SyncFileItemVector::iterator it_next = it + 1;
-                             it_next != _syncedItems.end() && it_next->_file.startsWith(path); ++it_next) {
+                             it_next != _syncedItems.end() && it_next->destination().startsWith(path); ++it_next) {
                             it = it_next;
                             it->_instruction = CSYNC_INSTRUCTION_ERROR;
                             it->_status = SyncFileItem::NormalError;