From 1944d9b3abb5f3f20fa5e2ba2004dbd7994bb13f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 20 Sep 2016 01:15:24 +0200 Subject: [PATCH 1/3] Use magic DI --- apps/dav/appinfo/app.php | 5 +- apps/dav/appinfo/register_command.php | 4 +- apps/dav/lib/AppInfo/Application.php | 104 ++---------------- apps/dav/lib/CalDAV/BirthdayService.php | 2 +- .../tests/unit/AppInfo/ApplicationTest.php | 6 +- 5 files changed, 17 insertions(+), 104 deletions(-) diff --git a/apps/dav/appinfo/app.php b/apps/dav/appinfo/app.php index ebc1d09e3f..0d417fd3fe 100644 --- a/apps/dav/appinfo/app.php +++ b/apps/dav/appinfo/app.php @@ -23,6 +23,7 @@ */ use OCA\DAV\AppInfo\Application; +use OCA\DAV\CardDAV\CardDavBackend; use Symfony\Component\EventDispatcher\GenericEvent; $app = new Application(); @@ -36,8 +37,8 @@ $eventDispatcher = \OC::$server->getEventDispatcher(); $eventDispatcher->addListener('OCP\Federation\TrustedServerEvent::remove', function(GenericEvent $event) use ($app) { - /** @var \OCA\DAV\CardDAV\CardDavBackend $cardDavBackend */ - $cardDavBackend = $app->getContainer()->query('CardDavBackend'); + /** @var CardDavBackend $cardDavBackend */ + $cardDavBackend = $app->getContainer()->query(CardDavBackend::class); $addressBookUri = $event->getSubject(); $addressBook = $cardDavBackend->getAddressBooksByUri('principals/system/system', $addressBookUri); if (!is_null($addressBook)) { diff --git a/apps/dav/appinfo/register_command.php b/apps/dav/appinfo/register_command.php index 7595c1b833..57bf8ff5dd 100644 --- a/apps/dav/appinfo/register_command.php +++ b/apps/dav/appinfo/register_command.php @@ -34,6 +34,6 @@ $app = new Application(); /** @var Symfony\Component\Console\Application $application */ $application->add(new CreateCalendar($userManager, $groupManager, $dbConnection)); -$application->add(new CreateAddressBook($userManager, $app->getContainer()->query('CardDavBackend'))); +$application->add(new CreateAddressBook($userManager, $app->getContainer()->query(\OCA\DAV\CardDAV\CardDavBackend::class))); $application->add(new SyncSystemAddressBook($app->getSyncService())); -$application->add(new SyncBirthdayCalendar($userManager, $app->getContainer()->query('BirthdayService'))); +$application->add(new SyncBirthdayCalendar($userManager, $app->getContainer()->query(\OCA\DAV\CalDAV\BirthdayService::class))); diff --git a/apps/dav/lib/AppInfo/Application.php b/apps/dav/lib/AppInfo/Application.php index 17145847fa..dc3ce7e8bb 100644 --- a/apps/dav/lib/AppInfo/Application.php +++ b/apps/dav/lib/AppInfo/Application.php @@ -25,110 +25,20 @@ namespace OCA\DAV\AppInfo; use OCA\DAV\CalDAV\BirthdayService; -use OCA\DAV\CalDAV\CalDavBackend; -use OCA\DAV\CardDAV\CardDavBackend; use OCA\DAV\CardDAV\ContactsManager; -use OCA\DAV\CardDAV\SyncJob; use OCA\DAV\CardDAV\SyncService; -use OCA\DAV\Connector\Sabre\Principal; -use OCA\DAV\DAV\GroupPrincipalBackend; use OCA\DAV\HookManager; -use OCA\DAV\Migration\Classification; -use OCA\DAV\Migration\GenerateBirthdays; use \OCP\AppFramework\App; -use OCP\AppFramework\IAppContainer; use OCP\Contacts\IManager; -use OCP\IUser; -use Sabre\VObject\Reader; use Symfony\Component\EventDispatcher\GenericEvent; class Application extends App { /** * Application constructor. - * - * @param array $urlParams */ - public function __construct (array $urlParams=array()) { - parent::__construct('dav', $urlParams); - - $container = $this->getContainer(); - $container->registerService('ContactsManager', function($c) { - /** @var IAppContainer $c */ - return new ContactsManager( - $c->query('CardDavBackend') - ); - }); - - $container->registerService('HookManager', function($c) { - /** @var IAppContainer $c */ - return new HookManager( - $c->getServer()->getUserManager(), - $c->query('SyncService'), - $c->query('CalDavBackend'), - $c->query('CardDavBackend') - ); - }); - - $container->registerService('SyncService', function($c) { - /** @var IAppContainer $c */ - return new SyncService( - $c->query('CardDavBackend'), - $c->getServer()->getUserManager(), - $c->getServer()->getLogger() - ); - }); - - $container->registerService('CardDavBackend', function($c) { - /** @var IAppContainer $c */ - $db = $c->getServer()->getDatabaseConnection(); - $dispatcher = $c->getServer()->getEventDispatcher(); - $principal = new Principal( - $c->getServer()->getUserManager(), - $c->getServer()->getGroupManager() - ); - return new CardDavBackend($db, $principal, $c->getServer()->getUserManager(), $dispatcher); - }); - - $container->registerService('CalDavBackend', function($c) { - /** @var IAppContainer $c */ - $db = $c->getServer()->getDatabaseConnection(); - $principal = new Principal( - $c->getServer()->getUserManager(), - $c->getServer()->getGroupManager() - ); - return new CalDavBackend($db, $principal, $c->getServer()->getUserManager()); - }); - - $container->registerService('BirthdayService', function($c) { - /** @var IAppContainer $c */ - $g = new GroupPrincipalBackend( - $c->getServer()->getGroupManager() - ); - return new BirthdayService( - $c->query('CalDavBackend'), - $c->query('CardDavBackend'), - $g - ); - }); - - $container->registerService('OCA\DAV\Migration\Classification', function ($c) { - /** @var IAppContainer $c */ - return new Classification( - $c->query('CalDavBackend'), - $c->getServer()->getUserManager() - ); - }); - - $container->registerService('OCA\DAV\Migration\GenerateBirthdays', function ($c) { - /** @var IAppContainer $c */ - /** @var BirthdayService $b */ - $b = $c->query('BirthdayService'); - return new GenerateBirthdays( - $b, - $c->getServer()->getUserManager() - ); - }); + public function __construct() { + parent::__construct('dav'); } /** @@ -137,20 +47,20 @@ class Application extends App { */ public function setupContactsProvider(IManager $contactsManager, $userID) { /** @var ContactsManager $cm */ - $cm = $this->getContainer()->query('ContactsManager'); + $cm = $this->getContainer()->query(ContactsManager::class); $urlGenerator = $this->getContainer()->getServer()->getURLGenerator(); $cm->setupContactsProvider($contactsManager, $userID, $urlGenerator); } public function registerHooks() { /** @var HookManager $hm */ - $hm = $this->getContainer()->query('HookManager'); + $hm = $this->getContainer()->query(HookManager::class); $hm->setup(); $listener = function($event) { if ($event instanceof GenericEvent) { /** @var BirthdayService $b */ - $b = $this->getContainer()->query('BirthdayService'); + $b = $this->getContainer()->query(BirthdayService::class); $b->onCardChanged( $event->getArgument('addressBookId'), $event->getArgument('cardUri'), @@ -165,7 +75,7 @@ class Application extends App { $dispatcher->addListener('\OCA\DAV\CardDAV\CardDavBackend::deleteCard', function($event) { if ($event instanceof GenericEvent) { /** @var BirthdayService $b */ - $b = $this->getContainer()->query('BirthdayService'); + $b = $this->getContainer()->query(BirthdayService::class); $b->onCardDeleted( $event->getArgument('addressBookId'), $event->getArgument('cardUri') @@ -175,7 +85,7 @@ class Application extends App { } public function getSyncService() { - return $this->getContainer()->query('SyncService'); + return $this->getContainer()->query(SyncService::class); } } diff --git a/apps/dav/lib/CalDAV/BirthdayService.php b/apps/dav/lib/CalDAV/BirthdayService.php index 31a6c73f48..ab2794e67d 100644 --- a/apps/dav/lib/CalDAV/BirthdayService.php +++ b/apps/dav/lib/CalDAV/BirthdayService.php @@ -50,7 +50,7 @@ class BirthdayService { * @param CardDavBackend $cardDavBackEnd * @param GroupPrincipalBackend $principalBackend */ - public function __construct($calDavBackEnd, $cardDavBackEnd, $principalBackend) { + public function __construct(CalDavBackend $calDavBackEnd, CardDavBackend $cardDavBackEnd, GroupPrincipalBackend $principalBackend) { $this->calDavBackEnd = $calDavBackEnd; $this->cardDavBackEnd = $cardDavBackEnd; $this->principalBackend = $principalBackend; diff --git a/apps/dav/tests/unit/AppInfo/ApplicationTest.php b/apps/dav/tests/unit/AppInfo/ApplicationTest.php index d5fb4e811d..8656ce9351 100644 --- a/apps/dav/tests/unit/AppInfo/ApplicationTest.php +++ b/apps/dav/tests/unit/AppInfo/ApplicationTest.php @@ -24,6 +24,8 @@ namespace OCA\DAV\Tests\unit\AppInfo; use OCA\DAV\AppInfo\Application; +use OCA\DAV\CardDAV\CardDavBackend; +use OCA\DAV\CardDAV\ContactsManager; use OCP\Contacts\IManager; use Test\TestCase; @@ -40,9 +42,9 @@ class ApplicationTest extends TestCase { $c = $app->getContainer(); // assert service instances in the container are properly setup - $s = $c->query('ContactsManager'); + $s = $c->query(ContactsManager::class); $this->assertInstanceOf('OCA\DAV\CardDAV\ContactsManager', $s); - $s = $c->query('CardDavBackend'); + $s = $c->query(CardDavBackend::class); $this->assertInstanceOf('OCA\DAV\CardDAV\CardDavBackend', $s); } From d9063b6141bd4c1c6754ac749427b8bc2711d2f6 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 20 Sep 2016 13:26:06 +0200 Subject: [PATCH 2/3] Use default value instead of throwing when the service could not be found --- lib/private/AppFramework/Utility/SimpleContainer.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index 5f637518f4..60496ca329 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -62,7 +62,16 @@ class SimpleContainer extends Container implements IContainer { $resolveName = $parameterClass->name; } - $parameters[] = $this->query($resolveName); + try { + $parameters[] = $this->query($resolveName); + } catch (\Exception $e) { + // Service not found, use the default value when available + if ($parameter->isDefaultValueAvailable()) { + $parameters[] = $parameter->getDefaultValue(); + } else { + throw $e; + } + } } return $class->newInstanceArgs($parameters); } From 94b0315d7aee7efce13fa1bcc34acf36a724fef9 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 20 Sep 2016 14:28:17 +0200 Subject: [PATCH 3/3] Fix more tests --- apps/dav/tests/unit/AppInfo/ApplicationTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/dav/tests/unit/AppInfo/ApplicationTest.php b/apps/dav/tests/unit/AppInfo/ApplicationTest.php index 8656ce9351..bb6cfe67b4 100644 --- a/apps/dav/tests/unit/AppInfo/ApplicationTest.php +++ b/apps/dav/tests/unit/AppInfo/ApplicationTest.php @@ -51,15 +51,15 @@ class ApplicationTest extends TestCase { public function testContactsManagerSetup() { $app = new Application(); $c = $app->getContainer(); - $c->registerService('CardDavBackend', function($c) { - $service = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')->disableOriginalConstructor()->getMock(); + $c->registerService(CardDavBackend::class, function() { + $service = $this->createMock(CardDavBackend::class); $service->method('getAddressBooksForUser')->willReturn([]); return $service; }); // assert setupContactsProvider() is proper - /** @var IManager | \PHPUnit_Framework_MockObject_MockObject $cm */ - $cm = $this->getMockBuilder('OCP\Contacts\IManager')->disableOriginalConstructor()->getMock(); + /** @var IManager|\PHPUnit_Framework_MockObject_MockObject $cm */ + $cm = $this->createMock(IManager::class); $app->setupContactsProvider($cm, 'xxx'); $this->assertTrue(true); }