Fix config validation (spatie#159)

* move config validation for the `items` key to the resolver to avoid exceptions during package installation

* update tests for config validation & items resolver

Author: patinthehat

src/Helpers/ConfigurationValidator.php

@@ -16,26 +16,21 @@ public static function validate(?array $feeds = null): void
                 throw InvalidConfiguration::unrecognizedFormat($name, $config['format']);
             }
 
-            if (! self::validateFeedItemsResolver($config['items'])) {
-                throw InvalidConfiguration::invalidItemsValue($name);
-            }
-
             if (! View::exists($config['view'] ?? 'feed::atom')) {
                 throw InvalidConfiguration::invalidView($name);
             }
         }
     }
 
-    protected static function validateFeedItemsResolver($resolver): bool
+    public static function validateResolver(string $feedName, $resolver): void
     {
-        if ($resolver === null) {
-            return true;
-        }
-
-        if ($resolver === '') {
-            return true;
+        if (! self::feedItemsResolverIsValid($resolver)) {
+            throw InvalidConfiguration::invalidItemsValue($feedName);
         }
+    }
 
+    protected static function feedItemsResolverIsValid($resolver): bool
+    {
         if (! is_string($resolver) && ! is_array($resolver)) {
             return false;
         }

src/Helpers/ResolveFeedItems.php

@@ -7,8 +7,10 @@
 
 class ResolveFeedItems
 {
-    public static function resolve($resolver): Collection
+    public static function resolve(string $feedName, $resolver): Collection
     {
+        ConfigurationValidator::validateResolver($feedName, $resolver);
+
         $newResolver = $resolver;
 
         if (is_array($resolver) && ! str_contains($resolver[0], '@')) {

src/Http/FeedController.php

@@ -18,7 +18,7 @@ public function __invoke()
 
         abort_unless($feed, 404);
 
-        $items = ResolveFeedItems::resolve($feed['items']);
+        $items = ResolveFeedItems::resolve($name, $feed['items']);
 
         return new Feed(
             $feed['title'],

tests/ConfigurationValidatorTest.php

@@ -18,7 +18,6 @@ public function it_validates_feed_formats()
             try {
                 ConfigurationValidator::validate([
                     'feed1' => [
-                        'items' => 'Spatie\Feed\Test\DummyRepository@getAll',
                         'view' => 'feed::rss',
                         'format' => $format,
                     ],
@@ -38,7 +37,6 @@ public function it_throws_an_exception_for_invalid_feed_types()
 
         ConfigurationValidator::validate([
             'feed1' => [
-                'items' => 'Spatie\Feed\Test\DummyRepository@getAll',
                 'view' => 'feed::rss',
                 'format' => 'test',
             ],
@@ -50,20 +48,14 @@ public function it_throws_an_exception_for_an_invalid_items_value()
     {
         $exceptionCounter = 0;
 
-        $invalidItems = [[], ['test']];
+        $invalidItems = [[], null, '', ['test']];
         $validItems = ['Model@getAll', ['App\\Model', 'getItems'], ['App\\Model', 'getItems', 'param1']];
 
         $items = array_merge($invalidItems, $validItems);
 
         foreach ($items as $itemsValue) {
             try {
-                ConfigurationValidator::validate([
-                    'feed1' => [
-                        'items' => $itemsValue,
-                        'view' => 'feed::rss',
-                        'format' => 'rss',
-                    ],
-                ]);
+                ConfigurationValidator::validateResolver('feed1', $itemsValue);
             } catch (InvalidConfiguration $e) {
                 $exceptionCounter++;
             }
@@ -82,7 +74,6 @@ public function it_throws_an_exception_for_an_invalid_view()
             try {
                 ConfigurationValidator::validate([
                     'feed1' => [
-                        'items' => 'Spatie\Feed\Test\DummyRepository@getAll',
                         'view' => $view,
                         'format' => 'json',
                     ],

tests/ResolveFeedItemsTest.php

@@ -9,31 +9,31 @@ class ResolveFeedItemsTest extends TestCase
     /** @test */
     public function it_resolves_a_resolver_class_and_method_string()
     {
-        $result = ResolveFeedItems::resolve('\Spatie\Feed\Test\DummyRepository@getAll');
+        $result = ResolveFeedItems::resolve('feed1', '\Spatie\Feed\Test\DummyRepository@getAll');
 
         $this->assertCount(5, $result);
     }
 
     /** @test */
     public function it_resolves_a_resolver_class_and_method_string_with_parameters()
     {
-        $result = ResolveFeedItems::resolve(['Spatie\Feed\Test\DummyRepository@getAllWithArguments', 'filter' => 'first']);
+        $result = ResolveFeedItems::resolve('feed1', ['Spatie\Feed\Test\DummyRepository@getAllWithArguments', 'filter' => 'first']);
 
         $this->assertCount(1, $result);
     }
 
     /** @test */
     public function it_resolves_a_resolver_class_and_method_tuple()
     {
-        $result = ResolveFeedItems::resolve([DummyRepository::class, 'getAll']);
+        $result = ResolveFeedItems::resolve('feed1', [DummyRepository::class, 'getAll']);
 
         $this->assertCount(5, $result);
     }
 
     /** @test */
     public function it_resolves_a_resolver_class_and_method_tuple_with_parameters()
     {
-        $result = ResolveFeedItems::resolve([DummyRepository::class, 'getAllWithArguments', 'filter' => 'first']);
+        $result = ResolveFeedItems::resolve('feed1', [DummyRepository::class, 'getAllWithArguments', 'filter' => 'first']);
 
         $this->assertCount(1, $result);
     }