From 1d26cb03899b6e0186a4fe57f9899450a519d45e Mon Sep 17 00:00:00 2001 From: Nico Hoffmann Date: Tue, 5 Mar 2019 21:55:21 +0100 Subject: [PATCH 1/5] Allow nested collections (#1400) --- src/Cms/Collections.php | 5 +++-- tests/Cms/CollectionsTest.php | 5 ++++- tests/Cms/fixtures/collections/nested/test.php | 7 +++++++ 3 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 tests/Cms/fixtures/collections/nested/test.php diff --git a/src/Cms/Collections.php b/src/Cms/Collections.php index 6c9861187e..6631895851 100644 --- a/src/Cms/Collections.php +++ b/src/Cms/Collections.php @@ -5,6 +5,7 @@ use Closure; use Kirby\Exception\NotFoundException; use Kirby\Toolkit\Controller; +use Kirby\Toolkit\Str; /** * Manages and loads all collections @@ -107,11 +108,11 @@ public static function load(App $app): self $collections = $app->extensions('collections'); $root = $app->root('collections'); - foreach (glob($root . '/*.php') as $file) { + foreach (glob($root . '/{,*/}*.php', GLOB_BRACE) as $file) { $collection = require $file; if (is_a($collection, 'Closure')) { - $name = pathinfo($file, PATHINFO_FILENAME); + $name = Str::between($file, $root . '/', '.php'); $collections[$name] = $collection; } } diff --git a/tests/Cms/CollectionsTest.php b/tests/Cms/CollectionsTest.php index 3f08f6182e..aa85ccd4e6 100644 --- a/tests/Cms/CollectionsTest.php +++ b/tests/Cms/CollectionsTest.php @@ -71,8 +71,11 @@ public function testLoad() ]); $collections = Collections::load($app); - $result = $collections->get('test'); + $result = $collections->get('test'); + $this->assertInstanceOf(Collection::class, $result); + + $result = $collections->get('nested/test'); $this->assertInstanceOf(Collection::class, $result); } } diff --git a/tests/Cms/fixtures/collections/nested/test.php b/tests/Cms/fixtures/collections/nested/test.php new file mode 100644 index 0000000000..4448ced1e0 --- /dev/null +++ b/tests/Cms/fixtures/collections/nested/test.php @@ -0,0 +1,7 @@ + Date: Tue, 5 Mar 2019 21:58:50 +0100 Subject: [PATCH 2/5] Better test for nested collections --- tests/Cms/CollectionsTest.php | 2 +- tests/Cms/fixtures/collections/nested/test.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Cms/CollectionsTest.php b/tests/Cms/CollectionsTest.php index aa85ccd4e6..460f8fd657 100644 --- a/tests/Cms/CollectionsTest.php +++ b/tests/Cms/CollectionsTest.php @@ -76,6 +76,6 @@ public function testLoad() $this->assertInstanceOf(Collection::class, $result); $result = $collections->get('nested/test'); - $this->assertInstanceOf(Collection::class, $result); + $this->assertEquals('a', $result); } } diff --git a/tests/Cms/fixtures/collections/nested/test.php b/tests/Cms/fixtures/collections/nested/test.php index 4448ced1e0..3cdaac4b48 100644 --- a/tests/Cms/fixtures/collections/nested/test.php +++ b/tests/Cms/fixtures/collections/nested/test.php @@ -3,5 +3,5 @@ use Kirby\Cms\Collection; return function () { - return new Collection(); + return 'a'; }; From c09517da4f429089da997dc99248d233743dd26c Mon Sep 17 00:00:00 2001 From: Nico Hoffmann Date: Wed, 6 Mar 2019 19:38:41 +0100 Subject: [PATCH 3/5] Refactor Collections class --- src/Cms/App.php | 2 +- src/Cms/Collections.php | 73 +++++++++++-------- tests/Cms/CollectionsTest.php | 66 ++++++----------- tests/Cms/fixtures/collections/rearranged.php | 5 ++ tests/Cms/fixtures/collections/string.php | 5 ++ 5 files changed, 78 insertions(+), 73 deletions(-) create mode 100644 tests/Cms/fixtures/collections/rearranged.php create mode 100644 tests/Cms/fixtures/collections/string.php diff --git a/src/Cms/App.php b/src/Cms/App.php index b5a03f112f..32185b538f 100644 --- a/src/Cms/App.php +++ b/src/Cms/App.php @@ -273,7 +273,7 @@ public function collection(string $name) */ public function collections(): Collections { - return $this->collections = $this->collections ?? Collections::load($this); + return $this->collections = $this->collections ?? new Collections; } /** diff --git a/src/Cms/Collections.php b/src/Cms/Collections.php index 6631895851..46aebd92cb 100644 --- a/src/Cms/Collections.php +++ b/src/Cms/Collections.php @@ -53,16 +53,6 @@ public function __call(string $name, array $arguments = []) return $this->get($name, ...$arguments); } - /** - * Creates a new Collections set - * - * @param array $collections - */ - public function __construct(array $collections = []) - { - $this->collections = $collections; - } - /** * Loads a collection by name if registered * @@ -72,17 +62,23 @@ public function __construct(array $collections = []) */ public function get(string $name, array $data = []) { - if (isset($this->cache[$name]) === true) { - return $this->cache[$name]; + // if not yet loaded + if (isset($this->collections[$name]) === false) { + $this->collections[$name] = $this->load($name); } - if (isset($this->collections[$name]) === false) { - return null; + // if not yet cached + if (isset($this->cache[$name]) === false) { + $controller = new Controller($this->collections[$name]); + $this->cache[$name] = $controller->call(null, $data); } - $controller = new Controller($this->collections[$name]); + // return cloned object + if (is_object($this->cache[$name]) === true) { + return clone $this->cache[$name]; + } - return $this->cache[$name] = $controller->call(null, $data); + return $this->cache[$name]; } /** @@ -93,30 +89,47 @@ public function get(string $name, array $data = []) */ public function has(string $name): bool { - return isset($this->collections[$name]) === true; + if (isset($this->collections[$name]) === true) { + return true; + } + + try { + $this->load($name); + return true; + } catch (NotFoundException $e) { + return false; + } } /** - * Loads collections from php files in a - * given directory. - * - * @param string $root - * @return self - */ - public static function load(App $app): self + * Loads collection from php file in a + * given directory or from plugin extension. + * + * @param string $name + * @return mixed + */ + public function load(string $name) { - $collections = $app->extensions('collections'); - $root = $app->root('collections'); + $kirby = App::instance(); - foreach (glob($root . '/{,*/}*.php', GLOB_BRACE) as $file) { + // first check for collection file + $file = $kirby->root('collections') . '/' . $name . '.php'; + + if (file_exists($file)) { $collection = require $file; if (is_a($collection, 'Closure')) { - $name = Str::between($file, $root . '/', '.php'); - $collections[$name] = $collection; + return $collection; } } - return new static($collections); + // fallback to collections from plugins + $collections = $kirby->extensions('collections'); + + if (isset($collections[$name]) === true) { + return $collections[$name]; + } + + throw new NotFoundException('The collection cannot be found'); } } diff --git a/tests/Cms/CollectionsTest.php b/tests/Cms/CollectionsTest.php index 460f8fd657..412bacebc5 100644 --- a/tests/Cms/CollectionsTest.php +++ b/tests/Cms/CollectionsTest.php @@ -4,29 +4,27 @@ class CollectionsTest extends TestCase { - public function testGet() + protected function _app() { - $collection = new Collection(); - $collections = new Collections([ - 'test' => function () use ($collection) { - return $collection; - } + return new App([ + 'roots' => [ + 'collections' => __DIR__ . '/fixtures/collections' + ] ]); - - $result = $collections->get('test'); + } + public function testGet() + { + $app = $this->_app(); + $collection = new Collection(); + $result = $app->collections()->get('test'); $this->assertEquals($collection, $result); } public function testGetWithData() { - $collections = new Collections([ - 'test' => function ($a, $b) { - return $a . $b; - } - ]); - - $result = $collections->get('test', [ + $app = $this->_app(); + $result = $app->collections()->get('string', [ 'a' => 'a', 'b' => 'b' ]); @@ -36,13 +34,8 @@ public function testGetWithData() public function testGetWithRearrangedData() { - $collections = new Collections([ - 'test' => function ($b, $a) { - return $a . $b; - } - ]); - - $result = $collections->get('test', [ + $app = $this->_app(); + $result = $app->collections()->get('rearranged', [ 'a' => 'a', 'b' => 'b' ]); @@ -52,30 +45,19 @@ public function testGetWithRearrangedData() public function testHas() { - $collections = new Collections([ - 'test' => function ($b, $a) { - return $a . $b; - } - ]); - - $this->assertTrue($collections->has('test')); - $this->assertFalse($collections->has('does-not-exist')); + $app= $this->_app(); + $this->assertTrue($app->collections()->has('test')); + $this->assertFalse($app->collections()->has('does-not-exist')); + $this->assertTrue($app->collections()->has('test')); } public function testLoad() { - $app = new App([ - 'roots' => [ - 'collections' => __DIR__ . '/fixtures/collections' - ] - ]); - - $collections = Collections::load($app); - - $result = $collections->get('test'); - $this->assertInstanceOf(Collection::class, $result); + $app = $this->_app(); + $result = $app->collections()->load('test'); + $this->assertInstanceOf(Collection::class, $result()); - $result = $collections->get('nested/test'); - $this->assertEquals('a', $result); + $result = $app->collections()->load('nested/test'); + $this->assertEquals('a', $result()); } } diff --git a/tests/Cms/fixtures/collections/rearranged.php b/tests/Cms/fixtures/collections/rearranged.php new file mode 100644 index 0000000000..21f8f1847f --- /dev/null +++ b/tests/Cms/fixtures/collections/rearranged.php @@ -0,0 +1,5 @@ + Date: Wed, 6 Mar 2019 19:40:13 +0100 Subject: [PATCH 4/5] Collections: Remove unused Str class --- src/Cms/Collections.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Cms/Collections.php b/src/Cms/Collections.php index 46aebd92cb..2609a58c21 100644 --- a/src/Cms/Collections.php +++ b/src/Cms/Collections.php @@ -5,7 +5,6 @@ use Closure; use Kirby\Exception\NotFoundException; use Kirby\Toolkit\Controller; -use Kirby\Toolkit\Str; /** * Manages and loads all collections From a065f1bdb9672343857653da18db59ce665bd87a Mon Sep 17 00:00:00 2001 From: Bastian Allgeier Date: Mon, 11 Mar 2019 11:50:39 +0100 Subject: [PATCH 5/5] Improve doc block comment and tests --- src/Cms/Collections.php | 12 ++++++------ tests/Cms/CollectionsTest.php | 8 ++++++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/Cms/Collections.php b/src/Cms/Collections.php index 2609a58c21..aacaf3fbbd 100644 --- a/src/Cms/Collections.php +++ b/src/Cms/Collections.php @@ -101,12 +101,12 @@ public function has(string $name): bool } /** - * Loads collection from php file in a - * given directory or from plugin extension. - * - * @param string $name - * @return mixed - */ + * Loads collection from php file in a + * given directory or from plugin extension. + * + * @param string $name + * @return mixed + */ public function load(string $name) { $kirby = App::instance(); diff --git a/tests/Cms/CollectionsTest.php b/tests/Cms/CollectionsTest.php index 412bacebc5..9467f7222f 100644 --- a/tests/Cms/CollectionsTest.php +++ b/tests/Cms/CollectionsTest.php @@ -12,6 +12,7 @@ protected function _app() ] ]); } + public function testGet() { $app = $this->_app(); @@ -60,4 +61,11 @@ public function testLoad() $result = $app->collections()->load('nested/test'); $this->assertEquals('a', $result()); } + + public function testLoadNested() + { + $app = $this->_app(); + $result = $app->collections()->load('nested/test'); + $this->assertEquals('a', $result()); + } }