Allow nested collections (#1400)

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;
     }
 
     /**

src/Cms/Collections.php

@@ -52,16 +52,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
      *
@@ -71,17 +61,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];
     }
 
     /**
@@ -92,30 +88,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.
+     * Loads collection from php file in a
+     * given directory or from plugin extension.
      *
-     * @param  string $root
-     * @return self
+     * @param  string $name
+     * @return mixed
      */
-    public static function load(App $app): self
+    public function load(string $name)
     {
-        $collections = $app->extensions('collections');
-        $root        = $app->root('collections');
+        $kirby = App::instance();
 
-        foreach (glob($root . '/*.php') 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 = pathinfo($file, PATHINFO_FILENAME);
-                $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');
     }
 }

tests/Cms/CollectionsTest.php

@@ -4,29 +4,28 @@
 
 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 +35,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,27 +46,26 @@ 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'
-            ]
-        ]);
+        $app = $this->_app();
+        $result = $app->collections()->load('test');
+        $this->assertInstanceOf(Collection::class, $result());
 
-        $collections = Collections::load($app);
-        $result      = $collections->get('test');
+        $result = $app->collections()->load('nested/test');
+        $this->assertEquals('a', $result());
+    }
 
-        $this->assertInstanceOf(Collection::class, $result);
+    public function testLoadNested()
+    {
+        $app = $this->_app();
+        $result = $app->collections()->load('nested/test');
+        $this->assertEquals('a', $result());
     }
 }

tests/Cms/fixtures/collections/nested/test.php

@@ -0,0 +1,7 @@
+<?php
+
+use Kirby\Cms\Collection;
+
+return function () {
+    return 'a';
+};

tests/Cms/fixtures/collections/rearranged.php

@@ -0,0 +1,5 @@
+<?php
+
+return function ($b, $a) {
+    return $a . $b;
+};

tests/Cms/fixtures/collections/string.php

@@ -0,0 +1,5 @@
+<?php
+
+return function ($a, $b) {
+    return $a . $b;
+};