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

Query the cache when checking if a node exists #22506

Merged
merged 5 commits into from Mar 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 24 additions & 9 deletions apps/files_trashbin/tests/trashbin.php
Expand Up @@ -86,11 +86,12 @@ public static function setUpBeforeClass() {
}



public static function tearDownAfterClass() {
// cleanup test user
$user = \OC::$server->getUserManager()->get(self::TEST_TRASHBIN_USER1);
if ($user !== null) { $user->delete(); }
if ($user !== null) {
$user->delete();
}

\OC::$server->getConfig()->setSystemValue('trashbin_retention_obligation', self::$rememberRetentionObligation);

Expand All @@ -109,6 +110,18 @@ protected function setUp() {
parent::setUp();

\OC::$server->getAppManager()->enableApp('files_trashbin');
$config = \OC::$server->getConfig();
$mockConfig = $this->getMock('\OCP\IConfig');
$mockConfig->expects($this->any())
->method('getSystemValue')
->will($this->returnCallback(function ($key, $default) use ($config) {
if ($key === 'filesystem_check_changes') {
return \OC\Files\Cache\Watcher::CHECK_ONCE;
} else {
return $config->getSystemValue($key, $default);
}
}));
$this->overwriteService('AllConfig', $mockConfig);

$this->trashRoot1 = '/' . self::TEST_TRASHBIN_USER1 . '/files_trashbin';
$this->trashRoot2 = '/' . self::TEST_TRASHBIN_USER2 . '/files_trashbin';
Expand All @@ -117,6 +130,7 @@ protected function setUp() {
}

protected function tearDown() {
$this->restoreService('AllConfig');
// disable trashbin to be able to properly clean up
\OC::$server->getAppManager()->disableApp('files_trashbin');

Expand All @@ -138,8 +152,8 @@ protected function tearDown() {
public function testExpireOldFiles() {

$currentTime = time();
$expireAt = $currentTime - 2*24*60*60;
$expiredDate = $currentTime - 3*24*60*60;
$expireAt = $currentTime - 2 * 24 * 60 * 60;
$expiredDate = $currentTime - 3 * 24 * 60 * 60;

// create some files
\OC\Files\Filesystem::file_put_contents('file1.txt', 'file1');
Expand Down Expand Up @@ -187,7 +201,7 @@ public function testExpireOldFilesShared() {

$currentTime = time();
$folder = "trashTest-" . $currentTime . '/';
$expiredDate = $currentTime - 3*24*60*60;
$expiredDate = $currentTime - 3 * 24 * 60 * 60;

// create some files
\OC\Files\Filesystem::mkdir($folder);
Expand Down Expand Up @@ -250,6 +264,7 @@ public function testExpireOldFilesShared() {

/**
* verify that the array contains the expected results
*
* @param OCP\Files\FileInfo[] $result
* @param string[] $expected
*/
Expand All @@ -265,7 +280,7 @@ private function verifyArray($result, $expected) {
}
if (!$found) {
// if we didn't found the expected file, something went wrong
$this->assertTrue(false, "can't find expected file '" . $expectedFile . "' in trash bin");
$this->assertTrue(false, "can't find expected file '" . $expectedFile . "' in trash bin");
}
}
}
Expand All @@ -281,7 +296,7 @@ private function manipulateDeleteTime($files, $trashRoot, $expireDate) {
// modify every second file
$counter = ($counter + 1) % 2;
if ($counter === 1) {
$source = $trashRoot . '/files/' . $file['name'].'.d'.$file['mtime'];
$source = $trashRoot . '/files/' . $file['name'] . '.d' . $file['mtime'];
$target = \OC\Files\Filesystem::normalizePath($trashRoot . '/files/' . $file['name'] . '.d' . $expireDate);
$this->rootView->rename($source, $target);
$file['mtime'] = $expireDate;
Expand Down Expand Up @@ -445,7 +460,7 @@ public function testRestoreFileFromTrashedSubfolder() {
$trashedFile = $filesInTrash[0];

$this->assertTrue(
OCA\Files_Trashbin\Trashbin::restore(
OCA\Files_Trashbin\Trashbin::restore(
'folder.d' . $trashedFile->getMtime() . '/file1.txt',
'file1.txt',
$trashedFile->getMtime()
Expand Down Expand Up @@ -639,7 +654,7 @@ public static function loginHelper($user, $create = false) {
if ($create) {
try {
\OC::$server->getUserManager()->createUser($user, $user);
} catch(\Exception $e) { // catch username is already being used from previous aborted runs
} catch (\Exception $e) { // catch username is already being used from previous aborted runs

}
}
Expand Down
15 changes: 15 additions & 0 deletions apps/files_versions/tests/versions.php
Expand Up @@ -74,6 +74,19 @@ public static function tearDownAfterClass() {
protected function setUp() {
parent::setUp();

$config = \OC::$server->getConfig();
$mockConfig = $this->getMock('\OCP\IConfig');
$mockConfig->expects($this->any())
->method('getSystemValue')
->will($this->returnCallback(function ($key, $default) use ($config) {
if ($key === 'filesystem_check_changes') {
return \OC\Files\Cache\Watcher::CHECK_ONCE;
} else {
return $config->getSystemValue($key, $default);
}
}));
$this->overwriteService('AllConfig', $mockConfig);

// clear hooks
\OC_Hook::clear();
\OC::registerShareHooks();
Expand All @@ -87,6 +100,8 @@ protected function setUp() {
}

protected function tearDown() {
$this->restoreService('AllConfig');

if ($this->rootView) {
$this->rootView->deleteAll(self::TEST_VERSIONS_USER . '/files/');
$this->rootView->deleteAll(self::TEST_VERSIONS_USER2 . '/files/');
Expand Down
32 changes: 14 additions & 18 deletions lib/private/files/node/folder.php
Expand Up @@ -90,19 +90,19 @@ public function getDirectoryListing() {

/**
* @param string $path
* @param array $info
* @param FileInfo $info
* @return File|Folder
*/
protected function createNode($path, $info = array()) {
if (!isset($info['mimetype'])) {
protected function createNode($path, FileInfo $info = null) {
if (is_null($info)) {
$isDir = $this->view->is_dir($path);
} else {
$isDir = $info['mimetype'] === 'httpd/unix-directory';
$isDir = $info->getType() === FileInfo::TYPE_FOLDER;
}
if ($isDir) {
return new Folder($this->root, $this->view, $path);
return new Folder($this->root, $this->view, $path, $info);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$info seems to be an array as per PHPDoc. Folder seems however to expect \OCP\Files\FileInfo. Could you adjust this so that static code analysers are happy?

} else {
return new File($this->root, $this->view, $path);
return new File($this->root, $this->view, $path, $info);
}
}

Expand Down Expand Up @@ -211,10 +211,9 @@ public function searchByTag($tag, $userId) {
private function searchCommon($method, $args) {
$files = array();
$rootLength = strlen($this->path);
/**
* @var \OC\Files\Storage\Storage $storage
*/
list($storage, $internalPath) = $this->view->resolvePath($this->path);
$mount = $this->root->getMount($this->path);
$storage = $mount->getStorage();
$internalPath = $mount->getInternalPath($this->path);
$internalPath = rtrim($internalPath, '/');
if ($internalPath !== '') {
$internalPath = $internalPath . '/';
Expand All @@ -229,7 +228,7 @@ private function searchCommon($method, $args) {
$result['internalPath'] = $result['path'];
$result['path'] = substr($result['path'], $internalRootLength);
$result['storage'] = $storage;
$files[] = $result;
$files[] = new \OC\Files\FileInfo($this->path . '/' . $result['path'], $storage, $result['internalPath'], $result, $mount);
}
}

Expand All @@ -245,17 +244,14 @@ private function searchCommon($method, $args) {
$result['internalPath'] = $result['path'];
$result['path'] = $relativeMountPoint . $result['path'];
$result['storage'] = $storage;
$files[] = $result;
$files[] = new \OC\Files\FileInfo($this->path . '/' . $result['path'], $storage, $result['internalPath'], $result, $mount);
}
}
}

$result = array();
foreach ($files as $file) {
$result[] = $this->createNode($this->normalizePath($this->path . '/' . $file['path']), $file);
}

return $result;
return array_map(function(FileInfo $file) {
return $this->createNode($file->getPath(), $file);
}, $files);
}

/**
Expand Down
17 changes: 9 additions & 8 deletions lib/private/files/node/root.php
Expand Up @@ -176,8 +176,9 @@ public function get($path) {
$path = $this->normalizePath($path);
if ($this->isValidPath($path)) {
$fullPath = $this->getFullPath($path);
if ($this->view->file_exists($fullPath)) {
return $this->createNode($fullPath);
$fileInfo = $this->view->getFileInfo($fullPath);
if ($fileInfo) {
return $this->createNode($fullPath, $fileInfo);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/owncloud/core/pull/22506/files#r55397361. createNode expects an array as per PHPDoc.

} else {
throw new NotFoundException($path);
}
Expand Down Expand Up @@ -336,18 +337,18 @@ public function getUserFolder($userId) {
$dir = '/' . $userId;
$folder = null;

if (!$this->nodeExists($dir)) {
$folder = $this->newFolder($dir);
} else {
try {
$folder = $this->get($dir);
} catch (NotFoundException $e) {
$folder = $this->newFolder($dir);
}

$dir = '/files';
if (!$folder->nodeExists($dir)) {
try {
$folder = $folder->get($dir);
} catch (NotFoundException $e) {
$folder = $folder->newFolder($dir);
\OC_Util::copySkeleton($userId, $folder);
} else {
$folder = $folder->get($dir);
}

return $folder;
Expand Down