Skip to content

Commit

Permalink
only collect shares inside the given path in transfer-ownership
Browse files Browse the repository at this point in the history
  • Loading branch information
karakayasemi committed Oct 4, 2019
1 parent cc3c4a4 commit 850f122
Show file tree
Hide file tree
Showing 3 changed files with 236 additions and 9 deletions.
25 changes: 16 additions & 9 deletions apps/files/lib/Command/TransferOwnership.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
use OC\Share20\ProviderFactory;
use OCP\Files\FileInfo;
use OCP\Files\Mount\IMountManager;
use OCP\ILogger;
use OCP\IUserManager;
use OCP\Share\IManager;
use OCP\Share\IShare;
Expand All @@ -55,9 +54,6 @@ class TransferOwnership extends Command {
/** @var Manager */
private $encryptionManager;

/** @var ILogger */
private $logger;

/** @var ProviderFactory */
private $shareProviderFactory;

Expand Down Expand Up @@ -85,12 +81,11 @@ class TransferOwnership extends Command {
/** @var string */
private $finalTarget;

public function __construct(IUserManager $userManager, IManager $shareManager, IMountManager $mountManager, Manager $encryptionManager, ILogger $logger, ProviderFactory $shareProviderFactory) {
public function __construct(IUserManager $userManager, IManager $shareManager, IMountManager $mountManager, Manager $encryptionManager, ProviderFactory $shareProviderFactory) {
$this->userManager = $userManager;
$this->shareManager = $shareManager;
$this->mountManager = $mountManager;
$this->encryptionManager = $encryptionManager;
$this->logger = $logger;
$this->shareProviderFactory = $shareProviderFactory;
parent::__construct();
}
Expand Down Expand Up @@ -135,7 +130,7 @@ protected function execute(InputInterface $input, OutputInterface $output) {
$this->inputPath = \ltrim($this->inputPath, '/');

// target user has to be ready
if (!\OC::$server->getEncryptionManager()->isReadyForUser($this->destinationUser)) {
if (!$this->encryptionManager->isReadyForUser($this->destinationUser)) {
$output->writeln("<error>The target user is not ready to accept files. The user has at least to be logged in once.</error>");
return 2;
}
Expand Down Expand Up @@ -196,7 +191,6 @@ protected function analyse(OutputInterface $output) {
$output->writeln("Analysing files of $this->sourceUser ...");
$progress = new ProgressBar($output);
$progress->start();
$self = $this;
$walkPath = "$this->sourceUser/files";
if (\strlen($this->inputPath) > 0) {
if ($this->inputPath !== "$this->sourceUser/files") {
Expand Down Expand Up @@ -257,11 +251,24 @@ private function collectUsersShares(OutputInterface $output) {
$offset = 0;
while (true) {
$sharePage = $this->shareManager->getSharesBy($this->sourceUser, $shareType, null, true, 50, $offset);
$progress->advance(\count($sharePage));
if (empty($sharePage)) {
break;
}

if (\strlen($this->inputPath)>1) {
$inputPathWithEndingSlash = \rtrim($this->inputPath, '/') . '/';
/**
* filter only shares within the source folder
*/
$sharePage = \array_filter($sharePage, function (IShare $share) use ($inputPathWithEndingSlash) {
/**
* trim sharePath, because `/` character trimmed with ltrim in inputPath
*/
$trimmedSharePath = \ltrim($share->getNode()->getPath(), '/');
return $trimmedSharePath === $this->inputPath || (\strpos($trimmedSharePath, $inputPathWithEndingSlash) === 0);
});
}
$progress->advance(\count($sharePage));
$this->shares = \array_merge($this->shares, $sharePage);
$offset += 50;
}
Expand Down
214 changes: 214 additions & 0 deletions apps/files/tests/Command/TransferOwnershipTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
<?php
/**
* @author Semih Serhat Karakaya <[email protected]>
*
* @copyright Copyright (c) 2019, ownCloud GmbH
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

namespace OCA\Files\Tests\Command;

use OC\Encryption\Manager;
use OC\Share20\ProviderFactory;
use OCA\Files\Command\TransferOwnership;
use OCP\Files\Mount\IMountManager;
use OCP\IUser;
use OCP\IUserManager;
use OCP\Share;
use OCP\Share\IManager;
use PHPUnit\Framework\MockObject\MockObject;
use Symfony\Component\Console\Tester\CommandTester;
use Test\TestCase;
use Test\Traits\UserTrait;

/**
* Class TransferOwnershipTest
*
* @group DB
*
* @package OCA\Files\Tests\Command
*/
class TransferOwnershipTest extends TestCase {
use UserTrait;

/**
* @var IUserManager
*/
private $userManager;

/**
* @var IManager
*/
private $shareManager;

/**
* @var IMountManager
*/
private $mountManager;

/**
* @var Manager | MockObject
*/
private $encryptionManager;

/**
* @var ProviderFactory
*/
private $providerFactory;

/**
* @var IUser
*/
private $sourceUser;

/**
* @var IUser
*/
private $targetUser;

/**
* @var CommandTester
*/
private $commandTester;

protected function setup() {
parent::setUp();
$this->userManager = \OC::$server->getUserManager();
$this->shareManager = \OC::$server->getShareManager();
$this->mountManager = \OC::$server->getMountManager();
$this->encryptionManager = $this->createMock(Manager::class);
$this->providerFactory = new ProviderFactory(\OC::$server);

$this->sourceUser = $this->createUser('source-user');
$this->targetUser = $this->createUser('target-user');
$this->loginAsUser('source-user');
$this->logout();
$this->loginAsUser('target-user');
$this->logout();
$this->createUser('share-receiver');
$this->createTestFilesForSourceUser();

$command = new TransferOwnership(
$this->userManager,
$this->shareManager,
$this->mountManager,
$this->encryptionManager,
$this->providerFactory
);
$this->commandTester = new CommandTester($command);
}
protected function tearDown() {
$this->tearDownUserTrait();
$this->shareManager->userDeleted('share-receiver');
$this->shareManager->userDeleted($this->sourceUser->getUID());
$this->shareManager->userDeleted($this->targetUser->getUID());
parent::tearDown();
}

/**
* Creates files and folder for source-user as the following tree:
*
* ├── file_in_user_root_folder
* ├── shared_file_in_user_root_folder (shared with share-receiver)
* ├── transfer
* │ ├── shared_file (shared with share-receiver)
* │ ├── test_file1
* │ ├── test_file2
* │ ├── sub_folder
* │ ├── shared_file_in_sub_folder (shared with share-receiver)
*/
private function createTestFilesForSourceUser() {
$userFolder = \OC::$server->getUserFolder($this->sourceUser->getUID());
$userFolder->newFile('file_in_user_root_folder');
$file = $userFolder->newFile('shared_file_in_user_root_folder');
$share = $this->shareManager->newShare();
$share->setNode($file)
->setSharedBy('source-user')
->setSharedWith('share-receiver')
->setShareType(Share::SHARE_TYPE_USER)
->setPermissions(19);
$this->shareManager->createShare($share);

$userFolder->newFolder('transfer');
$userFolder->newFile('transfer/test_file1');
$userFolder->newFile('transfer/test_file2');
$file = $userFolder->newFile('transfer/shared_file');
$share = $this->shareManager->newShare();
$share->setNode($file)
->setSharedBy('source-user')
->setSharedWith('share-receiver')
->setShareType(Share::SHARE_TYPE_USER)
->setPermissions(19);
$this->shareManager->createShare($share);

$userFolder->newFolder('transfer/sub_folder');
$file = $userFolder->newFile('transfer/sub_folder/shared_file_in_sub_folder');
$share = $this->shareManager->newShare();
$share->setNode($file)
->setSharedBy('source-user')
->setSharedWith('share-receiver')
->setShareType(Share::SHARE_TYPE_USER)
->setPermissions(19);
$this->shareManager->createShare($share);
}

public function testTransferAllFiles() {
$this->encryptionManager->method('isReadyForUser')->willReturn(true);
$input = [
'source-user' => $this->sourceUser->getUID(),
'destination-user' => $this->targetUser->getUID()
];
$this->commandTester->execute($input);
$output = $this->commandTester->getDisplay();

$this->assertContains('Transferring files to target-user', $output);
$sourceShares = $this->shareManager->getSharesBy($this->sourceUser->getUID(), Share::SHARE_TYPE_USER);
$targetShares = $this->shareManager->getSharesBy($this->targetUser->getUID(), Share::SHARE_TYPE_USER);
$this->assertCount(0, $sourceShares);
$this->assertCount(3, $targetShares);
}

public function folderPathProvider() {
return [
['transfer', 1, 2],
['transfer/sub_folder', 2, 1]
];
}

/**
* @dataProvider folderPathProvider
*
* @param string $path
* @param int $expectedSourceShareCount
* @param int $expectedTargetShareCount
*/
public function testTransferSpecificFolder($path, $expectedSourceShareCount, $expectedTargetShareCount) {
$this->encryptionManager->method('isReadyForUser')->willReturn(true);
$input = [
'source-user' => $this->sourceUser->getUID(),
'destination-user' => $this->targetUser->getUID(),
'--path' => $path
];
$this->commandTester->execute($input);
$output = $this->commandTester->getDisplay();

$this->assertContains('Transferring files to target-user', $output);
$sourceShares = $this->shareManager->getSharesBy($this->sourceUser->getUID(), Share::SHARE_TYPE_USER);
$targetShares = $this->shareManager->getSharesBy($this->targetUser->getUID(), Share::SHARE_TYPE_USER);
$this->assertCount($expectedSourceShareCount, $sourceShares);
$this->assertCount($expectedTargetShareCount, $targetShares);
}
}
6 changes: 6 additions & 0 deletions changelog/unreleased/36222
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: only collect shares inside the given path in files:transfer-ownership command

Even path argument given, files:transfer-ownership command was trying to transfer all shares of sourceUser.
This situation caused random errors. With this PR it will only collect shares inside the given path.

https://github.com/owncloud/core/pull/36222

0 comments on commit 850f122

Please sign in to comment.