From c5d5c7dc1249f75922f85b99fe1f1d3f799b2793 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 23 Apr 2018 09:50:13 +0200 Subject: [PATCH 1/6] Do not load calendar/addressbook plugins if not needed Fixes #6711 Based on https://github.com/owncloud/core/pull/30149/commits/f0fb21cf3563088284c56be4eac48c7d9725cb21 Signed-off-by: Roeland Jago Douma --- apps/dav/lib/Server.php | 44 ++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 7fbd7671e8d4d..ae8e2927fe291 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -78,7 +78,6 @@ public function __construct(IRequest $request, $baseUri) { $this->baseUri = $baseUri; $logger = \OC::$server->getLogger(); $dispatcher = \OC::$server->getEventDispatcher(); - $sendInvitations = \OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes'; $root = new RootCollection(); $this->server = new \OCA\DAV\Connector\Sabre\Server(new CachingTree($root)); @@ -137,24 +136,28 @@ public function __construct(IRequest $request, $baseUri) { $this->server->addPlugin($acl); // calendar plugins - $this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin()); - $this->server->addPlugin(new \Sabre\CalDAV\ICSExportPlugin()); - $this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin()); - if ($sendInvitations) { - $this->server->addPlugin(\OC::$server->query(\OCA\DAV\CalDAV\Schedule\IMipPlugin::class)); + if ($this->requestIsForSubtree('calendars')) { + $this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin()); + $this->server->addPlugin(new \Sabre\CalDAV\ICSExportPlugin()); + $this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin()); + if (\OC::$server->getConfig()->getAppValue('dav', 'sendInvitations', 'yes') === 'yes') { + $this->server->addPlugin(\OC::$server->query(\OCA\DAV\CalDAV\Schedule\IMipPlugin::class)); + } + $this->server->addPlugin(new \Sabre\CalDAV\Subscriptions\Plugin()); + $this->server->addPlugin(new \Sabre\CalDAV\Notifications\Plugin()); + $this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest())); + $this->server->addPlugin(new \OCA\DAV\CalDAV\Publishing\PublishPlugin( + \OC::$server->getConfig(), + \OC::$server->getURLGenerator() + )); } - $this->server->addPlugin(new \Sabre\CalDAV\Subscriptions\Plugin()); - $this->server->addPlugin(new \Sabre\CalDAV\Notifications\Plugin()); - $this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest())); - $this->server->addPlugin(new \OCA\DAV\CalDAV\Publishing\PublishPlugin( - \OC::$server->getConfig(), - \OC::$server->getURLGenerator() - )); // addressbook plugins - $this->server->addPlugin(new \OCA\DAV\CardDAV\Plugin()); - $this->server->addPlugin(new VCFExportPlugin()); - $this->server->addPlugin(new ImageExportPlugin(new PhotoCache(\OC::$server->getAppDataDir('dav-photocache')))); + if ($this->requestIsForSubtree('addressbooks')) { + $this->server->addPlugin(new \OCA\DAV\CardDAV\Plugin()); + $this->server->addPlugin(new VCFExportPlugin()); + $this->server->addPlugin(new ImageExportPlugin(new PhotoCache(\OC::$server->getAppDataDir('dav-photocache')))); + } // system tags plugins $this->server->addPlugin(new SystemTagPlugin( @@ -280,4 +283,13 @@ public function __construct(IRequest $request, $baseUri) { public function exec() { $this->server->exec(); } + + /** + * @param string $subTree + * @return bool + */ + private function requestIsForSubtree($subTree) { + $subTree = trim($subTree, " /"); + return strpos($this->server->getRequestUri(), "$subTree/") === 0; + } } From 240006bdf517df0a72d52efb285fdf9cf8b58d35 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 23 Apr 2018 13:48:39 +0200 Subject: [PATCH 2/6] When sharing calendars and addressbooks the principal has to be verified to be valid https://github.com/owncloud/core/pull/30149/commits/d3fb8fcdd3a6b00bde0c3c9eb4039876e7fc1967 Signed-off-by: Roeland Jago Douma --- apps/dav/lib/CalDAV/Calendar.php | 2 +- apps/dav/lib/CardDAV/AddressBook.php | 2 +- apps/dav/lib/Connector/Sabre/Principal.php | 7 +++++++ apps/dav/lib/DAV/Sharing/Backend.php | 12 +++++++++--- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/apps/dav/lib/CalDAV/Calendar.php b/apps/dav/lib/CalDAV/Calendar.php index 02808ab566275..a07bbe93218ca 100644 --- a/apps/dav/lib/CalDAV/Calendar.php +++ b/apps/dav/lib/CalDAV/Calendar.php @@ -203,7 +203,7 @@ public function delete() { } $this->caldavBackend->updateShares($this, [], [ - 'href' => $principal + $principal ]); return; } diff --git a/apps/dav/lib/CardDAV/AddressBook.php b/apps/dav/lib/CardDAV/AddressBook.php index a034f8b942607..7120231987455 100644 --- a/apps/dav/lib/CardDAV/AddressBook.php +++ b/apps/dav/lib/CardDAV/AddressBook.php @@ -181,7 +181,7 @@ public function delete() { } $this->carddavBackend->updateShares($this, [], [ - 'href' => $principal + $principal ]); return; } diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php index b2f57cf715ce4..b94b093ab50cc 100644 --- a/apps/dav/lib/Connector/Sabre/Principal.php +++ b/apps/dav/lib/Connector/Sabre/Principal.php @@ -324,6 +324,13 @@ function findByUri($uri, $principalPrefix) { return $this->principalPrefix . '/' . $user->getUID(); } } + if (substr($uri, 0, 10) === 'principal:') { + $principal = substr($uri, 10); + $principal = $this->getPrincipalByPath($principal); + if ($principal !== null) { + return $principal['uri']; + } + } return null; } diff --git a/apps/dav/lib/DAV/Sharing/Backend.php b/apps/dav/lib/DAV/Sharing/Backend.php index 87c094c6d623f..433d9db9c08cd 100644 --- a/apps/dav/lib/DAV/Sharing/Backend.php +++ b/apps/dav/lib/DAV/Sharing/Backend.php @@ -67,12 +67,18 @@ public function __construct(IDBConnection $db, IUserManager $userManager, IGroup * @param string[] $add * @param string[] $remove */ - public function updateShares($shareable, $add, $remove) { + public function updateShares(IShareable $shareable, array $add, array $remove) { foreach($add as $element) { - $this->shareWith($shareable, $element); + $principal = $this->principalBackend->findByUri($element['href'], ''); + if ($principal !== '') { + $this->shareWith($shareable, $element); + } } foreach($remove as $element) { - $this->unshare($shareable, $element); + $principal = $this->principalBackend->findByUri($element, ''); + if ($principal !== '') { + $this->unshare($shareable, $element); + } } } From 69e650d0e8d0ea38724d0d4afc16683403dad6e0 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 23 Apr 2018 13:54:58 +0200 Subject: [PATCH 3/6] CalDAV and CardDAV plugins need to be registered for the principals collection as well https://github.com/owncloud/core/pull/30149/commits/9f2e6431b88a9635b291b0e824abf74ba766616d Signed-off-by: Roeland Jago Douma --- apps/dav/lib/Server.php | 22 ++++++++++++---------- apps/dav/tests/unit/ServerTest.php | 22 +++++++++++++++++----- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index ae8e2927fe291..8297871115630 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -71,7 +71,7 @@ class Server { private $baseUri; /** @var Connector\Sabre\Server */ - private $server; + public $server; public function __construct(IRequest $request, $baseUri) { $this->request = $request; @@ -136,7 +136,7 @@ public function __construct(IRequest $request, $baseUri) { $this->server->addPlugin($acl); // calendar plugins - if ($this->requestIsForSubtree('calendars')) { + if ($this->requestIsForSubtree(['calendars', 'principals'])) { $this->server->addPlugin(new \OCA\DAV\CalDAV\Plugin()); $this->server->addPlugin(new \Sabre\CalDAV\ICSExportPlugin()); $this->server->addPlugin(new \OCA\DAV\CalDAV\Schedule\Plugin()); @@ -153,7 +153,8 @@ public function __construct(IRequest $request, $baseUri) { } // addressbook plugins - if ($this->requestIsForSubtree('addressbooks')) { + if ($this->requestIsForSubtree(['addressbooks', 'principals'])) { + $this->server->addPlugin(new DAV\Sharing\Plugin($authBackend, \OC::$server->getRequest())); $this->server->addPlugin(new \OCA\DAV\CardDAV\Plugin()); $this->server->addPlugin(new VCFExportPlugin()); $this->server->addPlugin(new ImageExportPlugin(new PhotoCache(\OC::$server->getAppDataDir('dav-photocache')))); @@ -284,12 +285,13 @@ public function exec() { $this->server->exec(); } - /** - * @param string $subTree - * @return bool - */ - private function requestIsForSubtree($subTree) { - $subTree = trim($subTree, " /"); - return strpos($this->server->getRequestUri(), "$subTree/") === 0; + private function requestIsForSubtree(array $subTrees): bool { + foreach ($subTrees as $subTree) { + $subTree = trim($subTree, ' /'); + if (strpos($this->server->getRequestUri(), $subTree.'/') === 0) { + return true; + } + } + return false; } } diff --git a/apps/dav/tests/unit/ServerTest.php b/apps/dav/tests/unit/ServerTest.php index 58c77c1b0ec10..986899a2107a2 100644 --- a/apps/dav/tests/unit/ServerTest.php +++ b/apps/dav/tests/unit/ServerTest.php @@ -38,12 +38,24 @@ */ class ServerTest extends \Test\TestCase { - public function test() { - /** @var IRequest $r */ + /** + * @dataProvider providesUris + */ + public function test($uri, array $plugins) { + /** @var IRequest | \PHPUnit_Framework_MockObject_MockObject $r */ $r = $this->createMock(IRequest::class); - $r->method('getRequestUri') - ->willReturn('/'); + $r->expects($this->any())->method('getRequestUri')->willReturn($uri); $s = new Server($r, '/'); - $this->assertInstanceOf('OCA\DAV\Server', $s); + $this->assertNotNull($s->server); + foreach ($plugins as $plugin) { + $this->assertNotNull($s->server->getPlugin($plugin)); + } + } + public function providesUris() { + return [ + 'principals' => ['principals/users/admin', ['caldav', 'oc-resource-sharing', 'carddav']], + 'calendars' => ['calendars/admin', ['caldav', 'oc-resource-sharing']], + 'addressbooks' => ['addressbooks/admin', ['carddav', 'oc-resource-sharing']], + ]; } } From 8bf8e92092a545bcbd448cf3b360fa2822035b6b Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 23 Apr 2018 14:02:53 +0200 Subject: [PATCH 4/6] Adding repair step which cleans shares of invalid principals https://github.com/owncloud/core/pull/30149/commits/edacf22fbce4deefc181c53dd84c5fb278a9a28d Signed-off-by: Roeland Jago Douma --- apps/dav/appinfo/info.xml | 3 +- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + apps/dav/lib/Repair/RemoveInvalidShares.php | 100 ++++++++++++++++++ .../unit/Repair/RemoveInvalidSharesTest.php | 70 ++++++++++++ 5 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 apps/dav/lib/Repair/RemoveInvalidShares.php create mode 100644 apps/dav/tests/unit/Repair/RemoveInvalidSharesTest.php diff --git a/apps/dav/appinfo/info.xml b/apps/dav/appinfo/info.xml index d31851fe17ee9..5e0847ff5d10d 100644 --- a/apps/dav/appinfo/info.xml +++ b/apps/dav/appinfo/info.xml @@ -5,7 +5,7 @@ WebDAV WebDAV endpoint WebDAV endpoint - 1.5.2 + 1.5.3 agpl owncloud.org DAV @@ -28,6 +28,7 @@ OCA\DAV\Migration\FixBirthdayCalendarComponent OCA\DAV\Migration\CalDAVRemoveEmptyValue OCA\DAV\Migration\BuildCalendarSearchIndex + OCA\DAV\Repair\RemoveInvalidShares diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 50689568ebb5c..d4ef0a5337a2d 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -142,6 +142,7 @@ 'OCA\\DAV\\Migration\\Version1004Date20170924124212' => $baseDir . '/../lib/Migration/Version1004Date20170924124212.php', 'OCA\\DAV\\Migration\\Version1004Date20170926103422' => $baseDir . '/../lib/Migration/Version1004Date20170926103422.php', 'OCA\\DAV\\Migration\\Version1005Date20180413093149' => $baseDir . '/../lib/Migration/Version1005Date20180413093149.php', + 'OCA\\DAV\\Repair\\RemoveInvalidShares' => $baseDir . '/../lib/Repair/RemoveInvalidShares.php', 'OCA\\DAV\\RootCollection' => $baseDir . '/../lib/RootCollection.php', 'OCA\\DAV\\Server' => $baseDir . '/../lib/Server.php', 'OCA\\DAV\\Settings\\CalDAVSettings' => $baseDir . '/../lib/Settings/CalDAVSettings.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 760ca3426f7dc..afaecd28e8e5f 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -157,6 +157,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Migration\\Version1004Date20170924124212' => __DIR__ . '/..' . '/../lib/Migration/Version1004Date20170924124212.php', 'OCA\\DAV\\Migration\\Version1004Date20170926103422' => __DIR__ . '/..' . '/../lib/Migration/Version1004Date20170926103422.php', 'OCA\\DAV\\Migration\\Version1005Date20180413093149' => __DIR__ . '/..' . '/../lib/Migration/Version1005Date20180413093149.php', + 'OCA\\DAV\\Repair\\RemoveInvalidShares' => __DIR__ . '/..' . '/../lib/Repair/RemoveInvalidShares.php', 'OCA\\DAV\\RootCollection' => __DIR__ . '/..' . '/../lib/RootCollection.php', 'OCA\\DAV\\Server' => __DIR__ . '/..' . '/../lib/Server.php', 'OCA\\DAV\\Settings\\CalDAVSettings' => __DIR__ . '/..' . '/../lib/Settings/CalDAVSettings.php', diff --git a/apps/dav/lib/Repair/RemoveInvalidShares.php b/apps/dav/lib/Repair/RemoveInvalidShares.php new file mode 100644 index 0000000000000..b69f918a57670 --- /dev/null +++ b/apps/dav/lib/Repair/RemoveInvalidShares.php @@ -0,0 +1,100 @@ + + * + * @copyright Copyright (c) 2018, 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 + * + */ + +namespace OCA\DAV\Repair; + +use OCA\DAV\Connector\Sabre\Principal; +use OCP\IDBConnection; +use OCP\ILogger; +use OCP\Migration\IOutput; +use OCP\Migration\IRepairStep; + +/** + * Class RemoveInvalidShares - removes shared calendars and addressbook which + * have no matching principal. Happened because of a bug in the calendar app. + * + * @package OCA\DAV\Repair + */ +class RemoveInvalidShares implements IRepairStep { + + /** @var IDBConnection */ + private $connection; + /** @var Principal */ + private $principalBackend; + + /** + * RemoveInvalidShares constructor. + * + * @param IDBConnection $connection + * @param Principal $principalBackend + */ + public function __construct(IDBConnection $connection, + Principal $principalBackend) { + $this->connection = $connection; + $this->principalBackend = $principalBackend; + } + + /** + * Returns the step's name + * + * @return string + * @since 9.1.0 + */ + public function getName() { + return 'Remove invalid calendar and addressbook shares'; + } + + /** + * Run repair step. + * Must throw exception on error. + * + * @param IOutput $output + * @throws \Exception in case of failure + * @since 9.1.0 + */ + public function run(IOutput $output) { + $query = $this->connection->getQueryBuilder(); + $result = $query->selectDistinct('principaluri') + ->from('dav_shares') + ->execute(); + + while($row = $result->fetch()) { + $principaluri = $row['principaluri']; + $p = $this->principalBackend->getPrincipalByPath($principaluri); + if ($p === null) { + $output->info(" ... for principal '$principaluri'"); + $this->deleteSharesForPrincipal($principaluri); + } + } + + $result->closeCursor(); + } + + /** + * @param string $principaluri + */ + private function deleteSharesForPrincipal($principaluri) { + $delete = $this->connection->getQueryBuilder(); + $delete->delete('dav_shares') + ->where($delete->expr()->eq('principaluri', $delete->createNamedParameter($principaluri))); + $delete->execute(); + } +} diff --git a/apps/dav/tests/unit/Repair/RemoveInvalidSharesTest.php b/apps/dav/tests/unit/Repair/RemoveInvalidSharesTest.php new file mode 100644 index 0000000000000..14553740f3810 --- /dev/null +++ b/apps/dav/tests/unit/Repair/RemoveInvalidSharesTest.php @@ -0,0 +1,70 @@ + + * + * @copyright Copyright (c) 2018, 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 + * + */ + + +namespace OCA\DAV\Tests\Unit\Repair; + + +use OCA\DAV\Connector\Sabre\Principal; +use OCA\DAV\Repair\RemoveInvalidShares; +use OCP\Migration\IOutput; +use Test\TestCase; + +/** + * Class RemoveInvalidSharesTest + * + * @package OCA\DAV\Tests\Unit\Repair + * @group DB + */ +class RemoveInvalidSharesTest extends TestCase { + + public function setUp() { + parent::setUp(); + $db = \OC::$server->getDatabaseConnection(); + + $db->insertIfNotExist('*PREFIX*dav_shares', [ + 'principaluri' => 'principal:unknown', + 'type' => 'calendar', + 'access' => 2, + 'resourceid' => 666, + ]); + } + + public function test() { + $db = \OC::$server->getDatabaseConnection(); + /** @var Principal | \PHPUnit_Framework_MockObject_MockObject $principal */ + $principal = $this->createMock(Principal::class); + + /** @var IOutput | \PHPUnit_Framework_MockObject_MockObject $output */ + $output = $this->createMock(IOutput::class); + + $repair = new RemoveInvalidShares($db, $principal); + $this->assertEquals("Remove invalid calendar and addressbook shares", $repair->getName()); + $repair->run($output); + + $query = $db->getQueryBuilder(); + $result = $query->select('*')->from('dav_shares') + ->where($query->expr()->eq('principaluri', $query->createNamedParameter('principal:unknown')))->execute(); + $data = $result->fetchAll(); + $result->closeCursor(); + $this->assertEquals(0, count($data)); + } +} From f9a64d3e1bf35c704fcf2ec9151b947213917a90 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 23 Apr 2018 14:29:46 +0200 Subject: [PATCH 5/6] Fix tests Signed-off-by: Roeland Jago Douma --- apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php | 9 ++++++++- apps/dav/tests/unit/CardDAV/CardDavBackendTest.php | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php index 310433f0913ce..681a159d833fd 100644 --- a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php +++ b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php @@ -30,7 +30,9 @@ use OCP\IGroupManager; use OCP\ILogger; use OCP\IUserManager; +use OCP\IUserSession; use OCP\Security\ISecureRandom; +use OCP\Share\IManager as ShareManager; use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Test\TestCase; @@ -73,7 +75,12 @@ public function setUp() { $this->groupManager = $this->createMock(IGroupManager::class); $this->dispatcher = $this->createMock(EventDispatcherInterface::class); $this->principal = $this->getMockBuilder(Principal::class) - ->disableOriginalConstructor() + ->setConstructorArgs([ + $this->userManager, + $this->groupManager, + $this->createMock(ShareManager::class), + $this->createMock(IUserSession::class), + ]) ->setMethods(['getPrincipalByPath', 'getGroupMembership']) ->getMock(); $this->principal->expects($this->any())->method('getPrincipalByPath') diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php index 63e090873bbf7..920e5a4ec1f43 100644 --- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php +++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php @@ -40,6 +40,8 @@ use OCP\IGroupManager; use OCP\IL10N; use OCP\IUserManager; +use OCP\IUserSession; +use OCP\Share\IManager as ShareManager; use Sabre\DAV\PropPatch; use Sabre\VObject\Component\VCard; use Sabre\VObject\Property\Text; @@ -90,7 +92,12 @@ public function setUp() { $this->userManager = $this->createMock(IUserManager::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->principal = $this->getMockBuilder(Principal::class) - ->disableOriginalConstructor() + ->setConstructorArgs([ + $this->userManager, + $this->groupManager, + $this->createMock(ShareManager::class), + $this->createMock(IUserSession::class), + ]) ->setMethods(['getPrincipalByPath', 'getGroupMembership']) ->getMock(); $this->principal->method('getPrincipalByPath') From 26c1e33d115cad3029c0663b7a388be0aff6417b Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 24 May 2018 09:27:40 +0200 Subject: [PATCH 6/6] Move repairstep to a custom command People that have issues can run it manually Signed-off-by: Roeland Jago Douma --- apps/dav/appinfo/info.xml | 2 +- .../composer/composer/autoload_classmap.php | 2 +- .../dav/composer/composer/autoload_static.php | 2 +- .../RemoveInvalidShares.php | 44 ++++++------------- .../RemoveInvalidSharesTest.php | 9 ++-- lib/private/Repair.php | 4 +- 6 files changed, 24 insertions(+), 39 deletions(-) rename apps/dav/lib/{Repair => Command}/RemoveInvalidShares.php (73%) rename apps/dav/tests/unit/{Repair => Command}/RemoveInvalidSharesTest.php (86%) diff --git a/apps/dav/appinfo/info.xml b/apps/dav/appinfo/info.xml index 5e0847ff5d10d..2dcb986c841a1 100644 --- a/apps/dav/appinfo/info.xml +++ b/apps/dav/appinfo/info.xml @@ -28,7 +28,6 @@ OCA\DAV\Migration\FixBirthdayCalendarComponent OCA\DAV\Migration\CalDAVRemoveEmptyValue OCA\DAV\Migration\BuildCalendarSearchIndex - OCA\DAV\Repair\RemoveInvalidShares @@ -37,6 +36,7 @@ OCA\DAV\Command\CreateCalendar OCA\DAV\Command\SyncBirthdayCalendar OCA\DAV\Command\SyncSystemAddressBook + OCA\DAV\Command\RemoveInvalidShares diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index d4ef0a5337a2d..075ee593ad728 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -65,6 +65,7 @@ 'OCA\\DAV\\CardDAV\\Xml\\Groups' => $baseDir . '/../lib/CardDAV/Xml/Groups.php', 'OCA\\DAV\\Command\\CreateAddressBook' => $baseDir . '/../lib/Command/CreateAddressBook.php', 'OCA\\DAV\\Command\\CreateCalendar' => $baseDir . '/../lib/Command/CreateCalendar.php', + 'OCA\\DAV\\Command\\RemoveInvalidShares' => $baseDir . '/../lib/Command/RemoveInvalidShares.php', 'OCA\\DAV\\Command\\SyncBirthdayCalendar' => $baseDir . '/../lib/Command/SyncBirthdayCalendar.php', 'OCA\\DAV\\Command\\SyncSystemAddressBook' => $baseDir . '/../lib/Command/SyncSystemAddressBook.php', 'OCA\\DAV\\Comments\\CommentNode' => $baseDir . '/../lib/Comments/CommentNode.php', @@ -142,7 +143,6 @@ 'OCA\\DAV\\Migration\\Version1004Date20170924124212' => $baseDir . '/../lib/Migration/Version1004Date20170924124212.php', 'OCA\\DAV\\Migration\\Version1004Date20170926103422' => $baseDir . '/../lib/Migration/Version1004Date20170926103422.php', 'OCA\\DAV\\Migration\\Version1005Date20180413093149' => $baseDir . '/../lib/Migration/Version1005Date20180413093149.php', - 'OCA\\DAV\\Repair\\RemoveInvalidShares' => $baseDir . '/../lib/Repair/RemoveInvalidShares.php', 'OCA\\DAV\\RootCollection' => $baseDir . '/../lib/RootCollection.php', 'OCA\\DAV\\Server' => $baseDir . '/../lib/Server.php', 'OCA\\DAV\\Settings\\CalDAVSettings' => $baseDir . '/../lib/Settings/CalDAVSettings.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index afaecd28e8e5f..e4938350aa462 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -80,6 +80,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\CardDAV\\Xml\\Groups' => __DIR__ . '/..' . '/../lib/CardDAV/Xml/Groups.php', 'OCA\\DAV\\Command\\CreateAddressBook' => __DIR__ . '/..' . '/../lib/Command/CreateAddressBook.php', 'OCA\\DAV\\Command\\CreateCalendar' => __DIR__ . '/..' . '/../lib/Command/CreateCalendar.php', + 'OCA\\DAV\\Command\\RemoveInvalidShares' => __DIR__ . '/..' . '/../lib/Command/RemoveInvalidShares.php', 'OCA\\DAV\\Command\\SyncBirthdayCalendar' => __DIR__ . '/..' . '/../lib/Command/SyncBirthdayCalendar.php', 'OCA\\DAV\\Command\\SyncSystemAddressBook' => __DIR__ . '/..' . '/../lib/Command/SyncSystemAddressBook.php', 'OCA\\DAV\\Comments\\CommentNode' => __DIR__ . '/..' . '/../lib/Comments/CommentNode.php', @@ -157,7 +158,6 @@ class ComposerStaticInitDAV 'OCA\\DAV\\Migration\\Version1004Date20170924124212' => __DIR__ . '/..' . '/../lib/Migration/Version1004Date20170924124212.php', 'OCA\\DAV\\Migration\\Version1004Date20170926103422' => __DIR__ . '/..' . '/../lib/Migration/Version1004Date20170926103422.php', 'OCA\\DAV\\Migration\\Version1005Date20180413093149' => __DIR__ . '/..' . '/../lib/Migration/Version1005Date20180413093149.php', - 'OCA\\DAV\\Repair\\RemoveInvalidShares' => __DIR__ . '/..' . '/../lib/Repair/RemoveInvalidShares.php', 'OCA\\DAV\\RootCollection' => __DIR__ . '/..' . '/../lib/RootCollection.php', 'OCA\\DAV\\Server' => __DIR__ . '/..' . '/../lib/Server.php', 'OCA\\DAV\\Settings\\CalDAVSettings' => __DIR__ . '/..' . '/../lib/Settings/CalDAVSettings.php', diff --git a/apps/dav/lib/Repair/RemoveInvalidShares.php b/apps/dav/lib/Command/RemoveInvalidShares.php similarity index 73% rename from apps/dav/lib/Repair/RemoveInvalidShares.php rename to apps/dav/lib/Command/RemoveInvalidShares.php index b69f918a57670..12a5ee43d478f 100644 --- a/apps/dav/lib/Repair/RemoveInvalidShares.php +++ b/apps/dav/lib/Command/RemoveInvalidShares.php @@ -1,4 +1,5 @@ * @@ -19,58 +20,40 @@ * */ -namespace OCA\DAV\Repair; +namespace OCA\DAV\Command; use OCA\DAV\Connector\Sabre\Principal; use OCP\IDBConnection; -use OCP\ILogger; -use OCP\Migration\IOutput; -use OCP\Migration\IRepairStep; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; /** * Class RemoveInvalidShares - removes shared calendars and addressbook which * have no matching principal. Happened because of a bug in the calendar app. - * - * @package OCA\DAV\Repair */ -class RemoveInvalidShares implements IRepairStep { +class RemoveInvalidShares extends Command { /** @var IDBConnection */ private $connection; /** @var Principal */ private $principalBackend; - /** - * RemoveInvalidShares constructor. - * - * @param IDBConnection $connection - * @param Principal $principalBackend - */ public function __construct(IDBConnection $connection, Principal $principalBackend) { + parent::__construct(); + $this->connection = $connection; $this->principalBackend = $principalBackend; } - /** - * Returns the step's name - * - * @return string - * @since 9.1.0 - */ - public function getName() { - return 'Remove invalid calendar and addressbook shares'; + protected function configure() { + $this + ->setName('dav:remove-invalid-shares') + ->setDescription('Remove invalid dav shares'); } - /** - * Run repair step. - * Must throw exception on error. - * - * @param IOutput $output - * @throws \Exception in case of failure - * @since 9.1.0 - */ - public function run(IOutput $output) { + protected function execute(InputInterface $input, OutputInterface $output) { $query = $this->connection->getQueryBuilder(); $result = $query->selectDistinct('principaluri') ->from('dav_shares') @@ -80,7 +63,6 @@ public function run(IOutput $output) { $principaluri = $row['principaluri']; $p = $this->principalBackend->getPrincipalByPath($principaluri); if ($p === null) { - $output->info(" ... for principal '$principaluri'"); $this->deleteSharesForPrincipal($principaluri); } } diff --git a/apps/dav/tests/unit/Repair/RemoveInvalidSharesTest.php b/apps/dav/tests/unit/Command/RemoveInvalidSharesTest.php similarity index 86% rename from apps/dav/tests/unit/Repair/RemoveInvalidSharesTest.php rename to apps/dav/tests/unit/Command/RemoveInvalidSharesTest.php index 14553740f3810..2574e4d0aecda 100644 --- a/apps/dav/tests/unit/Repair/RemoveInvalidSharesTest.php +++ b/apps/dav/tests/unit/Command/RemoveInvalidSharesTest.php @@ -20,12 +20,14 @@ */ -namespace OCA\DAV\Tests\Unit\Repair; +namespace OCA\DAV\Tests\Unit\Command; use OCA\DAV\Connector\Sabre\Principal; -use OCA\DAV\Repair\RemoveInvalidShares; +use OCA\DAV\Command\RemoveInvalidShares; use OCP\Migration\IOutput; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; /** @@ -57,8 +59,7 @@ public function test() { $output = $this->createMock(IOutput::class); $repair = new RemoveInvalidShares($db, $principal); - $this->assertEquals("Remove invalid calendar and addressbook shares", $repair->getName()); - $repair->run($output); + $this->invokePrivate($repair, 'run', [$this->createMock(InputInterface::class), $this->createMock(OutputInterface::class)]); $query = $db->getQueryBuilder(); $result = $query->select('*')->from('dav_shares') diff --git a/lib/private/Repair.php b/lib/private/Repair.php index 8746f1e6f2701..2774699344399 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -47,6 +47,8 @@ use OC\Repair\RepairInvalidShares; use OC\Template\JSCombiner; use OC\Template\SCSSCacher; +use OCA\DAV\Connector\Sabre\Principal; +use OCA\DAV\Repair\RemoveInvalidShares; use OCP\AppFramework\QueryException; use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; @@ -146,7 +148,7 @@ public static function getRepairSteps() { */ public static function getExpensiveRepairSteps() { return [ - new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager()) + new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager()), ]; }