Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't move files in cache twice, fixes renaming for objectstores #17641

Merged
merged 19 commits into from
Oct 19, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
aab226c
don't move files in cache twice, fixes renaming for objectstores
butonic Jul 14, 2015
a87368a
Skip checkupdate test for swift
icewind1991 Oct 12, 2015
ef17933
Add tests for double cache rename
icewind1991 Oct 12, 2015
0cdd46d
fix sabre connector tests when using a non local home storage
icewind1991 Oct 12, 2015
e7f7548
Fix shared storage tests for non local home storage
icewind1991 Oct 12, 2015
e46741c
detect object homestorage in share code
icewind1991 Oct 12, 2015
8efd037
Make shared folder size propagation test work with object home storage
icewind1991 Oct 12, 2015
a87b34a
dont assume home storage is local in trash test
icewind1991 Oct 13, 2015
0c6c36d
fix objectstore files having create permissions
icewind1991 Oct 13, 2015
54cea05
Fix preserving file ids when restoring a file with object storage
icewind1991 Oct 13, 2015
416da0d
fix delete orphan shares test with object home storage
icewind1991 Oct 14, 2015
22c5c19
handle versions expire for home storages with unlimited quota
icewind1991 Oct 14, 2015
d749b9a
Fix rename shared versions test
icewind1991 Oct 15, 2015
2e8232e
Fix trashbin handling of unknown/unlimited free space
icewind1991 Oct 15, 2015
e436442
Fix listing of trash files in test
icewind1991 Oct 15, 2015
d636bce
fix encryption migration test
icewind1991 Oct 15, 2015
b04e0de
Fix termination of the ceph docker
DeepDiver1975 Oct 16, 2015
fed3994
Fix termination of the ceph docker
DeepDiver1975 Oct 16, 2015
de55f6a
Fix error in stop script
DeepDiver1975 Oct 16, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions apps/dav/tests/unit/connector/sabre/file.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace Test\Connector\Sabre;

use OC\Files\Storage\Local;
use Test\HookHelper;
use OC\Files\Filesystem;
use OCP\Lock\ILockingProvider;
Expand Down Expand Up @@ -798,14 +799,16 @@ private function listPartFiles(\OC\Files\View $userView = null, $path = '') {
}
$files = [];
list($storage, $internalPath) = $userView->resolvePath($path);
$realPath = $storage->getSourcePath($internalPath);
$dh = opendir($realPath);
while (($file = readdir($dh)) !== false) {
if (substr($file, strlen($file) - 5, 5) === '.part') {
$files[] = $file;
if($storage instanceof Local) {
$realPath = $storage->getSourcePath($internalPath);
$dh = opendir($realPath);
while (($file = readdir($dh)) !== false) {
if (substr($file, strlen($file) - 5, 5) === '.part') {
$files[] = $file;
}
}
closedir($dh);
}
closedir($dh);
return $files;
}

Expand Down
27 changes: 26 additions & 1 deletion apps/encryption/tests/lib/MigrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ public function setUp() {
}

protected function createDummyShareKeys($uid) {
$this->loginAsUser($uid);

$this->view->mkdir($uid . '/files_encryption/keys/folder1/folder2/folder3/file3');
$this->view->mkdir($uid . '/files_encryption/keys/folder1/folder2/file2');
$this->view->mkdir($uid . '/files_encryption/keys/folder1/file.1');
Expand All @@ -87,13 +89,17 @@ protected function createDummyShareKeys($uid) {
}

protected function createDummyUserKeys($uid) {
$this->loginAsUser($uid);

$this->view->mkdir($uid . '/files_encryption/');
$this->view->mkdir('/files_encryption/public_keys');
$this->view->file_put_contents($uid . '/files_encryption/' . $uid . '.privateKey', 'privateKey');
$this->view->file_put_contents('/files_encryption/public_keys/' . $uid . '.publicKey', 'publicKey');
}

protected function createDummyFileKeys($uid) {
$this->loginAsUser($uid);

$this->view->mkdir($uid . '/files_encryption/keys/folder1/folder2/folder3/file3');
$this->view->mkdir($uid . '/files_encryption/keys/folder1/folder2/file2');
$this->view->mkdir($uid . '/files_encryption/keys/folder1/file.1');
Expand All @@ -105,6 +111,8 @@ protected function createDummyFileKeys($uid) {
}

protected function createDummyFiles($uid) {
$this->loginAsUser($uid);

$this->view->mkdir($uid . '/files/folder1/folder2/folder3/file3');
$this->view->mkdir($uid . '/files/folder1/folder2/file2');
$this->view->mkdir($uid . '/files/folder1/file.1');
Expand All @@ -116,6 +124,8 @@ protected function createDummyFiles($uid) {
}

protected function createDummyFilesInTrash($uid) {
$this->loginAsUser($uid);

$this->view->mkdir($uid . '/files_trashbin/keys/file1.d5457864');
$this->view->mkdir($uid . '/files_trashbin/keys/folder1.d7437648723/file2');
$this->view->file_put_contents($uid . '/files_trashbin/keys/file1.d5457864/' . self::TEST_ENCRYPTION_MIGRATION_USER1 . '.shareKey' , 'data');
Expand Down Expand Up @@ -165,6 +175,7 @@ public function testMigrateToNewFolderStructure() {

$this->createDummySystemWideKeys();

/** @var \PHPUnit_Framework_MockObject_MockObject|\OCA\Encryption\Migration $m */
$m = $this->getMockBuilder('OCA\Encryption\Migration')
->setConstructorArgs(
[
Expand All @@ -176,27 +187,38 @@ public function testMigrateToNewFolderStructure() {
)->setMethods(['getSystemMountPoints'])->getMock();

$m->expects($this->any())->method('getSystemMountPoints')
->willReturn([['mountpoint' => 'folder1'], ['mountpoint' => 'folder2']]);
->will($this->returnValue([['mountpoint' => 'folder1'], ['mountpoint' => 'folder2']]));

$m->reorganizeFolderStructure();
// even if it runs twice folder should always move only once
$m->reorganizeFolderStructure();

$this->loginAsUser(self::TEST_ENCRYPTION_MIGRATION_USER1);

$this->assertTrue(
$this->view->file_exists(
self::TEST_ENCRYPTION_MIGRATION_USER1 . '/files_encryption/' .
$this->moduleId . '/' . self::TEST_ENCRYPTION_MIGRATION_USER1 . '.publicKey')
);

$this->loginAsUser(self::TEST_ENCRYPTION_MIGRATION_USER2);

$this->assertTrue(
$this->view->file_exists(
self::TEST_ENCRYPTION_MIGRATION_USER2 . '/files_encryption/' .
$this->moduleId . '/' . self::TEST_ENCRYPTION_MIGRATION_USER2 . '.publicKey')
);

$this->loginAsUser(self::TEST_ENCRYPTION_MIGRATION_USER3);

$this->assertTrue(
$this->view->file_exists(
self::TEST_ENCRYPTION_MIGRATION_USER3 . '/files_encryption/' .
$this->moduleId . '/' . self::TEST_ENCRYPTION_MIGRATION_USER3 . '.publicKey')
);

$this->loginAsUser(self::TEST_ENCRYPTION_MIGRATION_USER1);

$this->assertTrue(
$this->view->file_exists(
'/files_encryption/' . $this->moduleId . '/systemwide_1.publicKey')
Expand All @@ -217,6 +239,8 @@ public function testMigrateToNewFolderStructure() {
}

protected function verifyFilesInTrash($uid) {
$this->loginAsUser($uid);

// share keys
$this->assertTrue(
$this->view->file_exists($uid . '/files_encryption/keys/files_trashbin/file1.d5457864/' . $this->moduleId . '/' . self::TEST_ENCRYPTION_MIGRATION_USER1 . '.shareKey')
Expand Down Expand Up @@ -244,6 +268,7 @@ protected function verifyFilesInTrash($uid) {
protected function verifyNewKeyPath($uid) {
// private key
if ($uid !== '') {
$this->loginAsUser($uid);
$this->assertTrue($this->view->file_exists($uid . '/files_encryption/' . $this->moduleId . '/'. $uid . '.privateKey'));
}
// file keys
Expand Down
7 changes: 6 additions & 1 deletion apps/files/tests/command/deleteorphanedfilestest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
namespace OCA\Files\Tests\Command;

use OCA\Files\Command\DeleteOrphanedFiles;
use OCP\Files\StorageNotAvailableException;

class DeleteOrphanedFilesTest extends \Test\TestCase {

Expand Down Expand Up @@ -110,7 +111,11 @@ public function testClearFiles() {

$this->assertCount(0, $this->getFile($fileInfo->getId()), 'Asserts that file gets cleaned up');

$view->unlink('files/test');
// since we deleted the storage it might throw a (valid) StorageNotAvailableException
try {
$view->unlink('files/test');
} catch (StorageNotAvailableException $e) {
}
}
}

11 changes: 7 additions & 4 deletions apps/files_sharing/tests/sharedstorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ protected function setUp() {
}

protected function tearDown() {
$this->view->unlink($this->folder);
$this->view->unlink($this->filename);
if ($this->view) {
$this->view->unlink($this->folder);
$this->view->unlink($this->filename);
}

\OC\Files\Filesystem::getLoader()->removeStorageWrapper('oc_trashbin');

Expand Down Expand Up @@ -85,8 +87,9 @@ function testParentOfMountPointIsGone() {
$this->assertFalse($user2View->is_dir($this->folder));

// delete the local folder
$fullPath = \OC_Config::getValue('datadirectory') . '/' . self::TEST_FILES_SHARING_API_USER2 . '/files/localfolder';
rmdir($fullPath);
/** @var \OC\Files\Storage\Storage $storage */
list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath('/' . self::TEST_FILES_SHARING_API_USER2 . '/files/localfolder');
$storage->rmdir($internalPath);

//enforce reload of the mount points
self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
Expand Down
10 changes: 4 additions & 6 deletions apps/files_sharing/tests/watcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,8 @@ function testFolderSizePropagationToOwnerStorage() {
$this->sharedCache->put('', array('mtime' => 10, 'storage_mtime' => 10, 'size' => '-1', 'mimetype' => 'httpd/unix-directory'));

// run the propagation code
$result = $this->sharedStorage->getWatcher()->checkUpdate('');

$this->assertTrue($result);
$this->sharedStorage->getWatcher()->checkUpdate('');
$this->sharedStorage->getCache()->correctFolderSize('');

// the owner's parent dirs must have increase size
$newSizes = self::getOwnerDirSizes('files/container/shareddir');
Expand Down Expand Up @@ -139,9 +138,8 @@ function testSubFolderSizePropagationToOwnerStorage() {
$this->sharedCache->put('subdir', array('mtime' => 10, 'storage_mtime' => 10, 'size' => $dataLen, 'mimetype' => 'text/plain'));

// run the propagation code
$result = $this->sharedStorage->getWatcher()->checkUpdate('subdir');

$this->assertTrue($result);
$this->sharedStorage->getWatcher()->checkUpdate('subdir');
$this->sharedStorage->getCache()->correctFolderSize('subdir');

// the owner's parent dirs must have increase size
$newSizes = self::getOwnerDirSizes('files/container/shareddir/subdir');
Expand Down
5 changes: 3 additions & 2 deletions apps/files_trashbin/lib/trashbin.php
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,9 @@ private static function calculateFreeSpace($trashbinSize, $user) {
if ($quota === null || $quota === 'none') {
$quota = \OC\Files\Filesystem::free_space('/');
$softQuota = false;
if ($quota === \OCP\Files\FileInfo::SPACE_UNKNOWN) {
$quota = 0;
// inf or unknown free space
if ($quota < 0) {
$quota = PHP_INT_MAX;
}
} else {
$quota = \OCP\Util::computerFileSize($quota);
Expand Down
30 changes: 17 additions & 13 deletions apps/files_trashbin/tests/trashbin.php
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,8 @@ public function testExpireOldFilesShared() {
// user2-1.txt should have been expired
$this->verifyArray($filesInTrashUser2AfterDelete, array('user2-2.txt', 'user1-4.txt'));

self::loginHelper(self::TEST_TRASHBIN_USER1);

// user1-1.txt and user1-3.txt should have been expired
$filesInTrashUser1AfterDelete = OCA\Files_Trashbin\Helper::getTrashFiles('/', self::TEST_TRASHBIN_USER1);

Expand Down Expand Up @@ -600,22 +602,24 @@ public function testRestoreFileIntoReadOnlySourceFolder() {

// delete source folder
list($storage, $internalPath) = $this->rootView->resolvePath('/' . self::TEST_TRASHBIN_USER1 . '/files/folder');
$folderAbsPath = $storage->getSourcePath($internalPath);
// make folder read-only
chmod($folderAbsPath, 0555);
if ($storage instanceof \OC\Files\Storage\Local) {
$folderAbsPath = $storage->getSourcePath($internalPath);
// make folder read-only
chmod($folderAbsPath, 0555);

$this->assertTrue(
OCA\Files_Trashbin\Trashbin::restore(
'file1.txt.d' . $trashedFile->getMtime(),
$trashedFile->getName(),
$trashedFile->getMtime()
)
);
$this->assertTrue(
OCA\Files_Trashbin\Trashbin::restore(
'file1.txt.d' . $trashedFile->getMtime(),
$trashedFile->getName(),
$trashedFile->getMtime()
)
);

$file = $userFolder->get('file1.txt');
$this->assertEquals('foo', $file->getContent());
$file = $userFolder->get('file1.txt');
$this->assertEquals('foo', $file->getContent());

chmod($folderAbsPath, 0755);
chmod($folderAbsPath, 0755);
}
}

/**
Expand Down
35 changes: 26 additions & 9 deletions apps/files_versions/lib/storage.php
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,20 @@ private static function copyFileContents($view, $path1, $path2) {
$view->lockFile($path1, ILockingProvider::LOCK_EXCLUSIVE);
$view->lockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);

$result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2);
// TODO add a proper way of overwriting a file while maintaining file ids
if ($storage1->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage') || $storage2->instanceOfStorage('\OC\Files\ObjectStore\ObjectStoreStorage')) {
$source = $storage1->fopen($internalPath1, 'r');
$target = $storage2->fopen($internalPath2, 'w');
list(, $result) = \OC_Helper::streamCopy($source, $target);
fclose($source);
fclose($target);

if ($result !== false) {
$storage1->unlink($internalPath1);
}
} else {
$result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2);
}

$view->unlockFile($path1, ILockingProvider::LOCK_EXCLUSIVE);
$view->unlockFile($path2, ILockingProvider::LOCK_EXCLUSIVE);
Expand Down Expand Up @@ -663,17 +676,21 @@ public static function expire($filename, $versionsSize = null, $offset = 0) {

// calculate available space for version history
// subtract size of files and current versions size from quota
if ($softQuota) {
$files_view = new \OC\Files\View('/'.$uid.'/files');
$rootInfo = $files_view->getFileInfo('/', false);
$free = $quota-$rootInfo['size']; // remaining free space for user
if ( $free > 0 ) {
$availableSpace = ($free * self::DEFAULTMAXSIZE / 100) - ($versionsSize + $offset); // how much space can be used for versions
if ($quota >= 0) {
if ($softQuota) {
$files_view = new \OC\Files\View('/' . $uid . '/files');
$rootInfo = $files_view->getFileInfo('/', false);
$free = $quota - $rootInfo['size']; // remaining free space for user
if ($free > 0) {
$availableSpace = ($free * self::DEFAULTMAXSIZE / 100) - ($versionsSize + $offset); // how much space can be used for versions
} else {
$availableSpace = $free - $versionsSize - $offset;
}
} else {
$availableSpace = $free - $versionsSize - $offset;
$availableSpace = $quota - $offset;
}
} else {
$availableSpace = $quota - $offset;
$availableSpace = PHP_INT_MAX;
}

$allVersions = Storage::getVersions($uid, $filename);
Expand Down
5 changes: 2 additions & 3 deletions apps/files_versions/tests/versions.php
Original file line number Diff line number Diff line change
Expand Up @@ -301,11 +301,10 @@ public function testRenameInSharedFolder() {
// execute rename hook of versions app
\OC\Files\Filesystem::rename('/folder1/test.txt', '/folder1/folder2/test.txt');


self::loginHelper(self::TEST_VERSIONS_USER2);

$this->runCommands();

self::loginHelper(self::TEST_VERSIONS_USER);

$this->assertFalse($this->rootView->file_exists($v1));
$this->assertFalse($this->rootView->file_exists($v2));

Expand Down
1 change: 1 addition & 0 deletions autotest.sh
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ function execute_tests {
fi

if [ "$PRIMARY_STORAGE_CONFIG" == "swift" ] ; then
cd ..
echo "Kill the swift docker"
tests/objectstore/stop-swift-ceph.sh
fi
Expand Down
16 changes: 9 additions & 7 deletions lib/private/files/cache/updater.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,15 @@ public function rename($source, $target) {

if ($sourceStorage && $targetStorage) {
$targetCache = $targetStorage->getCache($sourceInternalPath);
if ($targetCache->inCache($targetInternalPath)) {
$targetCache->remove($targetInternalPath);
}
if ($sourceStorage === $targetStorage) {
$targetCache->move($sourceInternalPath, $targetInternalPath);
} else {
$targetCache->moveFromCache($sourceStorage->getCache(), $sourceInternalPath, $targetInternalPath);
if ($sourceStorage->getCache($sourceInternalPath)->inCache($sourceInternalPath)) {
if ($targetCache->inCache($targetInternalPath)) {
$targetCache->remove($targetInternalPath);
}
if ($sourceStorage === $targetStorage) {
$targetCache->move($sourceInternalPath, $targetInternalPath);
} else {
$targetCache->moveFromCache($sourceStorage->getCache(), $sourceInternalPath, $targetInternalPath);
}
}

if (pathinfo($sourceInternalPath, PATHINFO_EXTENSION) !== pathinfo($targetInternalPath, PATHINFO_EXTENSION)) {
Expand Down
4 changes: 2 additions & 2 deletions lib/private/files/objectstore/objectstorestorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ public function touch($path, $mtime = null) {
'size' => 0,
'mtime' => $mtime,
'storage_mtime' => $mtime,
'permissions' => \OCP\Constants::PERMISSION_ALL,
'permissions' => \OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_CREATE,
);
$fileId = $this->getCache()->put($path, $stat);
try {
Expand All @@ -362,7 +362,7 @@ public function writeBack($tmpFile) {
if (empty($stat)) {
// create new file
$stat = array(
'permissions' => \OCP\Constants::PERMISSION_ALL,
'permissions' => \OCP\Constants::PERMISSION_ALL - \OCP\Constants::PERMISSION_CREATE,
);
}
// update stat with new data
Expand Down
4 changes: 3 additions & 1 deletion lib/private/share/share.php
Original file line number Diff line number Diff line change
Expand Up @@ -2636,7 +2636,9 @@ public static function getExpireInterval() {
*/
private static function isFileReachable($path, $ownerStorageId) {
// if outside the home storage, file is always considered reachable
if (!(substr($ownerStorageId, 0, 6) === 'home::')) {
if (!(substr($ownerStorageId, 0, 6) === 'home::' ||
substr($ownerStorageId, 0, 13) === 'object::user:'
)) {
return true;
}

Expand Down
Loading