From 970846624feb154800666c7c3293d9ec27861c71 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 16 Mar 2015 16:17:43 +0100 Subject: [PATCH 1/2] Allow registering closures for navigation entries This speeds up all pages that don't use the navigation by 0.04sec per app, because we don't need to create the routing anymore, unless we really need to. --- lib/private/navigationmanager.php | 19 +++++++++++++++++-- lib/public/inavigationmanager.php | 7 +++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/private/navigationmanager.php b/lib/private/navigationmanager.php index 8ad2f4c8f6..205ecccd50 100644 --- a/lib/private/navigationmanager.php +++ b/lib/private/navigationmanager.php @@ -14,13 +14,23 @@ namespace OC; */ class NavigationManager implements \OCP\INavigationManager { protected $entries = array(); + protected $closureEntries = array(); protected $activeEntry; /** * Creates a new navigation entry - * @param array $entry containing: id, name, order, icon and href key + * + * @param array|\Closure $entry Array containing: id, name, order, icon and href key + * The use of a closure is preferred, because it will avoid + * loading the routing of your app, unless required. + * @return void */ - public function add(array $entry) { + public function add($entry) { + if ($entry instanceof \Closure) { + $this->closureEntries[] = $entry; + return; + } + $entry['active'] = false; if(!isset($entry['icon'])) { $entry['icon'] = ''; @@ -33,6 +43,10 @@ class NavigationManager implements \OCP\INavigationManager { * @return array an array of the added entries */ public function getAll() { + foreach ($this->closureEntries as $c) { + $this->add($c()); + } + $this->closureEntries = array(); return $this->entries; } @@ -41,6 +55,7 @@ class NavigationManager implements \OCP\INavigationManager { */ public function clear() { $this->entries = array(); + $this->closureEntries = array(); } /** diff --git a/lib/public/inavigationmanager.php b/lib/public/inavigationmanager.php index 9497d3fd08..9e0e682645 100644 --- a/lib/public/inavigationmanager.php +++ b/lib/public/inavigationmanager.php @@ -36,10 +36,13 @@ namespace OCP; interface INavigationManager { /** * Creates a new navigation entry - * @param array $entry containing: id, name, order, icon and href key + * + * @param array|\Closure $entry Array containing: id, name, order, icon and href key + * The use of a closure is preferred, because it will avoid + * loading the routing of your app, unless required. * @return void */ - public function add(array $entry); + public function add($entry); /** * Sets the current navigation entry of the currently running app From ec1d5011b6056f7281c09f3d3eac19a01e5fe928 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 16 Mar 2015 16:46:17 +0100 Subject: [PATCH 2/2] Add tests for the navigation manager (closure) behaviour --- tests/lib/navigationmanagertest.php | 160 ++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 tests/lib/navigationmanagertest.php diff --git a/tests/lib/navigationmanagertest.php b/tests/lib/navigationmanagertest.php new file mode 100644 index 0000000000..96708786e3 --- /dev/null +++ b/tests/lib/navigationmanagertest.php @@ -0,0 +1,160 @@ +navigationManager = new NavigationManager(); + } + + public function addArrayData() { + return [ + [ + [ + 'id' => 'entry id', + 'name' => 'link text', + 'order' => 1, + 'icon' => 'optional', + 'href' => 'url', + ], + [ + 'id' => 'entry id', + 'name' => 'link text', + 'order' => 1, + 'icon' => 'optional', + 'href' => 'url', + 'active' => false, + ], + ], + [ + [ + 'id' => 'entry id', + 'name' => 'link text', + 'order' => 1, + //'icon' => 'optional', + 'href' => 'url', + 'active' => true, + ], + [ + 'id' => 'entry id', + 'name' => 'link text', + 'order' => 1, + 'icon' => '', + 'href' => 'url', + 'active' => false, + ], + ], + ]; + } + + /** + * @dataProvider addArrayData + * + * @param array $entry + * @param array $expectedEntry + */ + public function testAddArray(array $entry, array $expectedEntry) { + $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists'); + $this->navigationManager->add($entry); + + $navigationEntries = $this->navigationManager->getAll(); + $this->assertEquals(1, sizeof($navigationEntries), 'Expected that 1 navigation entry exists'); + $this->assertEquals($expectedEntry, $navigationEntries[0]); + + $this->navigationManager->clear(); + $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists after clear()'); + } + + /** + * @dataProvider addArrayData + * + * @param array $entry + * @param array $expectedEntry + */ + public function testAddClosure(array $entry, array $expectedEntry) { + global $testAddClosureNumberOfCalls; + $testAddClosureNumberOfCalls = 0; + + $this->navigationManager->add(function () use ($entry) { + global $testAddClosureNumberOfCalls; + $testAddClosureNumberOfCalls++; + + return $entry; + }); + + $this->assertEquals(0, $testAddClosureNumberOfCalls, 'Expected that the closure is not called by add()'); + + $navigationEntries = $this->navigationManager->getAll(); + $this->assertEquals(1, $testAddClosureNumberOfCalls, 'Expected that the closure is called by getAll()'); + $this->assertEquals(1, sizeof($navigationEntries), 'Expected that 1 navigation entry exists'); + $this->assertEquals($expectedEntry, $navigationEntries[0]); + + $navigationEntries = $this->navigationManager->getAll(); + $this->assertEquals(1, $testAddClosureNumberOfCalls, 'Expected that the closure is only called once for getAll()'); + $this->assertEquals(1, sizeof($navigationEntries), 'Expected that 1 navigation entry exists'); + $this->assertEquals($expectedEntry, $navigationEntries[0]); + + $this->navigationManager->clear(); + $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists after clear()'); + } + + public function testAddArrayClearGetAll() { + $entry = [ + 'id' => 'entry id', + 'name' => 'link text', + 'order' => 1, + 'icon' => 'optional', + 'href' => 'url', + ]; + + $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists'); + $this->navigationManager->add($entry); + $this->navigationManager->clear(); + $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists after clear()'); + } + + public function testAddClosureClearGetAll() { + $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists'); + + $entry = [ + 'id' => 'entry id', + 'name' => 'link text', + 'order' => 1, + 'icon' => 'optional', + 'href' => 'url', + ]; + + global $testAddClosureNumberOfCalls; + $testAddClosureNumberOfCalls = 0; + + $this->navigationManager->add(function () use ($entry) { + global $testAddClosureNumberOfCalls; + $testAddClosureNumberOfCalls++; + + return $entry; + }); + + $this->assertEquals(0, $testAddClosureNumberOfCalls, 'Expected that the closure is not called by add()'); + $this->navigationManager->clear(); + $this->assertEquals(0, $testAddClosureNumberOfCalls, 'Expected that the closure is not called by clear()'); + $this->assertEmpty($this->navigationManager->getAll(), 'Expected no navigation entry exists after clear()'); + $this->assertEquals(0, $testAddClosureNumberOfCalls, 'Expected that the closure is not called by getAll()'); + } +}