catch excexptions during the copy operation and make sure that we free the lock correctly
This commit is contained in:
parent
67231ed9a7
commit
17a64360e5
2 changed files with 98 additions and 42 deletions
|
@ -759,59 +759,72 @@ class View {
|
|||
|
||||
$this->lockFile($path2, ILockingProvider::LOCK_SHARED);
|
||||
$this->lockFile($path1, ILockingProvider::LOCK_SHARED);
|
||||
$lockTypePath1 = ILockingProvider::LOCK_SHARED;
|
||||
$lockTypePath2 = ILockingProvider::LOCK_SHARED;
|
||||
|
||||
$exists = $this->file_exists($path2);
|
||||
if ($this->shouldEmitHooks()) {
|
||||
\OC_Hook::emit(
|
||||
Filesystem::CLASSNAME,
|
||||
Filesystem::signal_copy,
|
||||
array(
|
||||
Filesystem::signal_param_oldpath => $this->getHookPath($path1),
|
||||
Filesystem::signal_param_newpath => $this->getHookPath($path2),
|
||||
Filesystem::signal_param_run => &$run
|
||||
)
|
||||
);
|
||||
$this->emit_file_hooks_pre($exists, $path2, $run);
|
||||
}
|
||||
if ($run) {
|
||||
$mount1 = $this->getMount($path1);
|
||||
$mount2 = $this->getMount($path2);
|
||||
$storage1 = $mount1->getStorage();
|
||||
$internalPath1 = $mount1->getInternalPath($absolutePath1);
|
||||
$storage2 = $mount2->getStorage();
|
||||
$internalPath2 = $mount2->getInternalPath($absolutePath2);
|
||||
try {
|
||||
|
||||
$this->changeLock($path2, ILockingProvider::LOCK_EXCLUSIVE);
|
||||
|
||||
if ($mount1->getMountPoint() == $mount2->getMountPoint()) {
|
||||
if ($storage1) {
|
||||
$result = $storage1->copy($internalPath1, $internalPath2);
|
||||
} else {
|
||||
$result = false;
|
||||
}
|
||||
} else {
|
||||
$result = $storage2->copyFromStorage($storage1, $internalPath1, $internalPath2);
|
||||
}
|
||||
|
||||
$this->updater->update($path2);
|
||||
|
||||
$this->changeLock($path2, ILockingProvider::LOCK_SHARED);
|
||||
|
||||
if ($this->shouldEmitHooks() && $result !== false) {
|
||||
$exists = $this->file_exists($path2);
|
||||
if ($this->shouldEmitHooks()) {
|
||||
\OC_Hook::emit(
|
||||
Filesystem::CLASSNAME,
|
||||
Filesystem::signal_post_copy,
|
||||
Filesystem::signal_copy,
|
||||
array(
|
||||
Filesystem::signal_param_oldpath => $this->getHookPath($path1),
|
||||
Filesystem::signal_param_newpath => $this->getHookPath($path2)
|
||||
Filesystem::signal_param_newpath => $this->getHookPath($path2),
|
||||
Filesystem::signal_param_run => &$run
|
||||
)
|
||||
);
|
||||
$this->emit_file_hooks_post($exists, $path2);
|
||||
$this->emit_file_hooks_pre($exists, $path2, $run);
|
||||
}
|
||||
if ($run) {
|
||||
$mount1 = $this->getMount($path1);
|
||||
$mount2 = $this->getMount($path2);
|
||||
$storage1 = $mount1->getStorage();
|
||||
$internalPath1 = $mount1->getInternalPath($absolutePath1);
|
||||
$storage2 = $mount2->getStorage();
|
||||
$internalPath2 = $mount2->getInternalPath($absolutePath2);
|
||||
|
||||
$this->unlockFile($path2, ILockingProvider::LOCK_SHARED);
|
||||
$this->unlockFile($path1, ILockingProvider::LOCK_SHARED);
|
||||
$this->changeLock($path2, ILockingProvider::LOCK_EXCLUSIVE);
|
||||
$lockTypePath2 = ILockingProvider::LOCK_EXCLUSIVE;
|
||||
|
||||
if ($mount1->getMountPoint() == $mount2->getMountPoint()) {
|
||||
if ($storage1) {
|
||||
$result = $storage1->copy($internalPath1, $internalPath2);
|
||||
} else {
|
||||
$result = false;
|
||||
}
|
||||
} else {
|
||||
$result = $storage2->copyFromStorage($storage1, $internalPath1, $internalPath2);
|
||||
}
|
||||
|
||||
$this->updater->update($path2);
|
||||
|
||||
$this->changeLock($path2, ILockingProvider::LOCK_SHARED);
|
||||
$lockTypePath2 = ILockingProvider::LOCK_SHARED;
|
||||
|
||||
if ($this->shouldEmitHooks() && $result !== false) {
|
||||
\OC_Hook::emit(
|
||||
Filesystem::CLASSNAME,
|
||||
Filesystem::signal_post_copy,
|
||||
array(
|
||||
Filesystem::signal_param_oldpath => $this->getHookPath($path1),
|
||||
Filesystem::signal_param_newpath => $this->getHookPath($path2)
|
||||
)
|
||||
);
|
||||
$this->emit_file_hooks_post($exists, $path2);
|
||||
}
|
||||
|
||||
}
|
||||
} catch (\Exception $e) {
|
||||
$this->unlockFile($path2, $lockTypePath2);
|
||||
$this->unlockFile($path1, $lockTypePath1);
|
||||
throw $e;
|
||||
}
|
||||
|
||||
$this->unlockFile($path2, $lockTypePath2);
|
||||
$this->unlockFile($path1, $lockTypePath1);
|
||||
|
||||
}
|
||||
return $result;
|
||||
}
|
||||
|
|
|
@ -1817,6 +1817,49 @@ class View extends \Test\TestCase {
|
|||
$this->assertNull($this->getFileLockType($view, $targetPath), 'Target file not locked after operation');
|
||||
}
|
||||
|
||||
/**
|
||||
* simulate a failed copy operation.
|
||||
* We expect that we catch the exception, free the lock and re-throw it.
|
||||
*
|
||||
* @expectedException \Exception
|
||||
*/
|
||||
public function testLockFileCopyException() {
|
||||
$view = new \OC\Files\View('/' . $this->user . '/files/');
|
||||
|
||||
$storage = $this->getMockBuilder('\OC\Files\Storage\Temporary')
|
||||
->setMethods(['copy'])
|
||||
->getMock();
|
||||
|
||||
$sourcePath = 'original.txt';
|
||||
$targetPath = 'target.txt';
|
||||
|
||||
\OC\Files\Filesystem::mount($storage, array(), $this->user . '/');
|
||||
$storage->mkdir('files');
|
||||
$view->file_put_contents($sourcePath, 'meh');
|
||||
|
||||
$storage->expects($this->once())
|
||||
->method('copy')
|
||||
->will($this->returnCallback(
|
||||
function() {
|
||||
throw new \Exception();
|
||||
}
|
||||
));
|
||||
|
||||
$this->connectMockHooks('copy', $view, $sourcePath, $lockTypeSourcePre, $lockTypeSourcePost);
|
||||
$this->connectMockHooks('copy', $view, $targetPath, $lockTypeTargetPre, $lockTypeTargetPost);
|
||||
|
||||
$this->assertNull($this->getFileLockType($view, $sourcePath), 'Source file not locked before operation');
|
||||
$this->assertNull($this->getFileLockType($view, $targetPath), 'Target file not locked before operation');
|
||||
|
||||
try {
|
||||
$view->copy($sourcePath, $targetPath);
|
||||
} catch (\Exception $e) {
|
||||
$this->assertNull($this->getFileLockType($view, $sourcePath), 'Source file not locked after operation');
|
||||
$this->assertNull($this->getFileLockType($view, $targetPath), 'Target file not locked after operation');
|
||||
throw $e;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Test rename operation: unlock first path when second path was locked
|
||||
*/
|
||||
|
|
Loading…
Reference in a new issue