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

Add a command to poll incoming federated shares for updates #34903

Merged
merged 3 commits into from
Mar 29, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/federatedfilesharing/appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,7 @@
<personal>OCA\FederatedFileSharing\Panels\GeneralPersonalPanel</personal>
<personal>OCA\FederatedFileSharing\Panels\SharingPersonalPanel</personal>
</settings>
<commands>
<command>OCA\FederatedFileSharing\Command\PollIncomingShares</command>
</commands>
</info>
20 changes: 20 additions & 0 deletions apps/federatedfilesharing/lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
namespace OCA\FederatedFileSharing\AppInfo;

use OCA\FederatedFileSharing\AddressHandler;
use OCA\FederatedFileSharing\Command\PollIncomingShares;
use OCA\FederatedFileSharing\Controller\OcmController;
use OCA\FederatedFileSharing\DiscoveryManager;
use OCA\FederatedFileSharing\FederatedShareProvider;
Expand Down Expand Up @@ -157,6 +158,25 @@ function ($c) {
return new Permissions();
}
);

$container->registerService(
PollIncomingShares::class,
function ($c) use ($server) {
if ($server->getAppManager()->isEnabledForUser('files_sharing')) {
$sharingApp = new \OCA\Files_Sharing\AppInfo\Application();
$externalMountProvider = $sharingApp->getContainer()->query('ExternalMountProvider');
} else {
$externalMountProvider = null;
}

return new PollIncomingShares(
$server->getDatabaseConnection(),
$server->getUserManager(),
\OC\Files\Filesystem::getLoader(),
$externalMountProvider
);
}
);
}

/**
Expand Down
127 changes: 127 additions & 0 deletions apps/federatedfilesharing/lib/Command/PollIncomingShares.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
<?php
/**
* @author Viktar Dubiniuk <dubiniuk@owncloud.com>
*
* @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\FederatedFileSharing\Command;

use OC\ServerNotAvailableException;
use OCA\Files_Sharing\External\MountProvider;
use OCP\Files\Storage\IStorage;
use OCP\Files\Storage\IStorageFactory;
use OCP\Files\StorageInvalidException;
use OCP\Files\StorageNotAvailableException;
use OCP\IDBConnection;
use OCP\IUserManager;
use OCP\Lock\LockedException;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

class PollIncomingShares extends Command {
/** @var IDBConnection */
private $dbConnection;

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

/** @var IStorageFactory */
private $loader;

/** @var MountProvider | null */
private $externalMountProvider;

/**
* PollIncomingShares constructor.
*
* @param IDBConnection $dbConnection
* @param IUserManager $userManager
* @param MountProvider $externalMountProvider
* @param IStorageFactory $loader
*/
public function __construct(IDBConnection $dbConnection, IUserManager $userManager, IStorageFactory $loader, MountProvider $externalMountProvider = null) {
parent::__construct();
$this->dbConnection = $dbConnection;
$this->userManager = $userManager;
$this->loader = $loader;
$this->externalMountProvider = $externalMountProvider;
}

protected function configure() {
$this->setName('incoming-shares:poll')
->setDescription('Poll incoming federated shares manually to detect updates');
}

/**
* @param InputInterface $input
* @param OutputInterface $output
*
* @return int|null|void
*/
public function execute(InputInterface $input, OutputInterface $output) {
if ($this->externalMountProvider === null) {
$output->writeln("Polling is not possible when files_sharing app is disabled. Please enable it with 'occ app:enable files_sharing'");
return 1;
}
$cursor = $this->getCursor();
while ($data = $cursor->fetch()) {
$user = $this->userManager->get($data['user']);
$userMounts = $this->externalMountProvider->getMountsForUser($user, $this->loader);
/** @var \OCA\Files_Sharing\External\Mount $mount */
foreach ($userMounts as $mount) {
try {
/** @var Storage $storage */
$storage = $mount->getStorage();
$this->refreshStorageRoot($storage);
} catch (\Exception $e) {
$output->writeln($e->getMessage());
Copy link
Contributor

Choose a reason for hiding this comment

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

Problem here, is exceptions never get logged and since this command is designed probably to run in cron, will go unnoticed. Would suggest to log as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

@tomneedham at least some of them are logged by the dav app:

deo@reborn:~/public_html/oc-master> rm data/owncloud.log
deo@reborn:~/public_html/oc-master> php occ incoming-shares:poll
Sabre\HTTP\ClientHttpException: Unauthorized
deo@reborn:~/public_html/oc-master> cat data/owncloud.log          
{"reqId":"0jcYYTjvA7asrGLBtk0R","level":3,"time":"2019-03-29T10:07:45+00:00","remoteAddr":"","user":"--","app":"no app in context","method":"--","url":"--","message":"Exception: {\"Exception\":\"Sabre\\\\HTTP\\\\ClientHttpException\",\"Message\":\"Unauthorized\",\"Code\":401,\"Trace\":\"#0 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Client.php(232): Sabre\\\\HTTP\\\\Client->send(Object(Sabre\\\\HTTP\\\\Request))\\n#1 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php(259): Sabre\\\\DAV\\\\Client->propFind('http:\\\/\\\/localhos...', Array)\\n#2 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php(579): OC\\\\Files\\\\Storage\\\\DAV->propfind('')\\n#3 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Common.php(175): OC\\\\Files\\\\Storage\\\\DAV->stat('')\\n#4 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Common->filemtime('')\\n#5 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Availability.php(252): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#6 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Availability->filemtime('')\\n#7 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#8 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/apps\\\/federatedfilesharing\\\/lib\\\/Command\\\/PollIncomingShares.php(110): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#9 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/apps\\\/federatedfilesharing\\\/lib\\\/Command\\\/PollIncomingShares.php(92): OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares->refreshStorageRoot(Object(OCA\\\\Files_Trashbin\\\\Storage))\\n#10 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Command\\\/Command.php(255): OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares->execute(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#11 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(960): Symfony\\\\Component\\\\Console\\\\Command\\\\Command->run(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#12 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(255): Symfony\\\\Component\\\\Console\\\\Application->doRunCommand(Object(OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares), Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#13 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(148): Symfony\\\\Component\\\\Console\\\\Application->doRun(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#14 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Console\\\/Application.php(164): Symfony\\\\Component\\\\Console\\\\Application->run(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#15 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/console.php(106): OC\\\\Console\\\\Application->run()\\n#16 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/occ(11): require_once('\\\/home\\\/deo\\\/publi...')\\n#17 {main}\",\"File\":\"\\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/sabre\\\/http\\\/lib\\\/Client.php\",\"Line\":155}"}
{"reqId":"0jcYYTjvA7asrGLBtk0R","level":3,"time":"2019-03-29T10:07:45+00:00","remoteAddr":"","user":"--","app":"files_external","method":"--","url":"--","message":"Unauthorized"}
{"reqId":"0jcYYTjvA7asrGLBtk0R","level":3,"time":"2019-03-29T10:07:45+00:00","remoteAddr":"","user":"--","app":"no app in context","method":"--","url":"--","message":"Exception: {\"Exception\":\"OCP\\\\Files\\\\StorageInvalidException\",\"Message\":\"Sabre\\\\HTTP\\\\ClientHttpException: Unauthorized\",\"Code\":0,\"Trace\":\"#0 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php(830): OC\\\\Files\\\\Storage\\\\DAV->throwByStatusCode(401, Object(Sabre\\\\HTTP\\\\ClientHttpException), '')\\n#1 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php(275): OC\\\\Files\\\\Storage\\\\DAV->convertException(Object(Sabre\\\\HTTP\\\\ClientHttpException), '')\\n#2 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php(579): OC\\\\Files\\\\Storage\\\\DAV->propfind('')\\n#3 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Common.php(175): OC\\\\Files\\\\Storage\\\\DAV->stat('')\\n#4 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Common->filemtime('')\\n#5 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Availability.php(252): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#6 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Availability->filemtime('')\\n#7 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/Wrapper\\\/Wrapper.php(232): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#8 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/apps\\\/federatedfilesharing\\\/lib\\\/Command\\\/PollIncomingShares.php(110): OC\\\\Files\\\\Storage\\\\Wrapper\\\\Wrapper->filemtime('')\\n#9 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/apps\\\/federatedfilesharing\\\/lib\\\/Command\\\/PollIncomingShares.php(92): OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares->refreshStorageRoot(Object(OCA\\\\Files_Trashbin\\\\Storage))\\n#10 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Command\\\/Command.php(255): OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares->execute(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#11 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(960): Symfony\\\\Component\\\\Console\\\\Command\\\\Command->run(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#12 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(255): Symfony\\\\Component\\\\Console\\\\Application->doRunCommand(Object(OCA\\\\FederatedFileSharing\\\\Command\\\\PollIncomingShares), Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#13 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/composer\\\/symfony\\\/console\\\/Application.php(148): Symfony\\\\Component\\\\Console\\\\Application->doRun(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#14 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Console\\\/Application.php(164): Symfony\\\\Component\\\\Console\\\\Application->run(Object(Symfony\\\\Component\\\\Console\\\\Input\\\\ArgvInput), Object(Symfony\\\\Component\\\\Console\\\\Output\\\\ConsoleOutput))\\n#15 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/console.php(106): OC\\\\Console\\\\Application->run()\\n#16 \\\/home\\\/deo\\\/public_html\\\/oc-master\\\/occ(11): require_once('\\\/home\\\/deo\\\/publi...')\\n#17 {main}\",\"File\":\"\\\/home\\\/deo\\\/public_html\\\/oc-master\\\/lib\\\/private\\\/Files\\\/Storage\\\/DAV.php\",\"Line\":865}"}
{"reqId":"0jcYYTjvA7asrGLBtk0R","level":3,"time":"2019-03-29T10:07:45+00:00","remoteAddr":"","user":"--","app":"files_external","method":"--","url":"--","message":"Sabre\\HTTP\\ClientHttpException: Unauthorized"}

Copy link
Contributor

Choose a reason for hiding this comment

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

it's likely logged on a higher level already ?

}
}
}
$cursor->closeCursor();
}

/**
* @param IStorage $storage
*
* @throws LockedException
* @throws ServerNotAvailableException
* @throws StorageInvalidException
* @throws StorageNotAvailableException
*/
protected function refreshStorageRoot(IStorage $storage) {
$localMtime = $storage->filemtime('');
/** @var \OCA\Files_Sharing\External\Storage $storage */
if ($storage->hasUpdated('', $localMtime)) {
$storage->getScanner('')->scan('', false, 0);
}
}

/**
* @return \Doctrine\DBAL\Driver\Statement
*/
protected function getCursor() {
Copy link
Member

Choose a reason for hiding this comment

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

Move this to the FederatedShareProvider? It should be fine to inject the provider directly in order to use a public method that isn't in the interface.

Copy link
Member Author

Choose a reason for hiding this comment

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

My taste tells me that it should stay here.
Could be moved to the provider but it needs to be renamed into getUniqueRecipients and return them as an array of uids (and this array may be huuuge)

Copy link
Member

Choose a reason for hiding this comment

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

If you don't want to leak DB stuff, you may return a generator instead of an array. As long as it's clear that the function returns a generator of uids I think it's fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the current code is good enough

$qb = $this->dbConnection->getQueryBuilder();
$qb->selectDistinct('user')
->from('share_external')
->where($qb->expr()->eq('accepted', $qb->expr()->literal('1')));

return $qb->execute();
}
}
100 changes: 100 additions & 0 deletions apps/federatedfilesharing/tests/Command/PollIncomingSharesTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
<?php
/**
* @author Viktar Dubiniuk <dubiniuk@owncloud.com>
*
* @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\FederatedFileSharing\Tests\Command;

use Doctrine\DBAL\Driver\Statement;
use OCA\FederatedFileSharing\Tests\TestCase;
use OCA\FederatedFileSharing\Command\PollIncomingShares;
use OCA\Files_Sharing\External\MountProvider;
use OCP\DB\QueryBuilder\IExpressionBuilder;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Files\Storage\IStorageFactory;
use OCP\IDBConnection;
use OCP\IUser;
use OCP\IUserManager;
use Symfony\Component\Console\Tester\CommandTester;

/**
* Class PollIncomingSharesTest
*
* @group DB
* @package OCA\FederatedFileSharing\Tests\Command
*/
class PollIncomingSharesTest extends TestCase {
/** @var CommandTester */
private $commandTester;

/** @var IDBConnection | \PHPUnit_Framework_MockObject_MockObject */
private $dbConnection;

/** @var IUserManager | \PHPUnit_Framework_MockObject_MockObject */
private $userManager;

/** @var MountProvider | \PHPUnit_Framework_MockObject_MockObject */
private $externalMountProvider;

/** @var IStorageFactory | \PHPUnit_Framework_MockObject_MockObject */
private $loader;

protected function setUp() {
parent::setUp();
$this->dbConnection = $this->createMock(IDBConnection::class);
$this->userManager = $this->createMock(IUserManager::class);
$this->externalMountProvider = $this->createMock(MountProvider::class);
$this->loader = $this->createMock(IStorageFactory::class);
$command = new PollIncomingShares($this->dbConnection, $this->userManager, $this->loader, $this->externalMountProvider);
$this->commandTester = new CommandTester($command);
}

public function testNoSharesPoll() {
$uid = 'foo';
$exprBuilder = $this->createMock(IExpressionBuilder::class);
$statementMock = $this->createMock(Statement::class);
$statementMock->method('fetch')->willReturnOnConsecutiveCalls(['user' => $uid], false);
$qbMock = $this->createMock(IQueryBuilder::class);
$qbMock->method('selectDistinct')->willReturnSelf();
$qbMock->method('from')->willReturnSelf();
$qbMock->method('where')->willReturnSelf();
$qbMock->method('expr')->willReturn($exprBuilder);
$qbMock->method('execute')->willReturn($statementMock);

$userMock = $this->createMock(IUser::class);
$this->userManager->expects($this->once())->method('get')
->with($uid)->willReturn($userMock);

$this->externalMountProvider->expects($this->once())->method('getMountsForUser')
->willReturn([]);

$this->dbConnection->method('getQueryBuilder')->willReturn($qbMock);
$this->commandTester->execute([]);
$output = $this->commandTester->getDisplay();
$this->assertEmpty($output);
}

public function testWithFilesSharingDisabled() {
$command = new PollIncomingShares($this->dbConnection, $this->userManager, $this->loader, null);
$this->commandTester = new CommandTester($command);
$this->commandTester->execute([]);
$output = $this->commandTester->getDisplay();
$this->assertContains("Polling is not possible when files_sharing app is disabled. Please enable it with 'occ app:enable files_sharing'", $output);
}
}