diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 53fc95b9911..980fdd8e6dd 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -22,6 +22,7 @@ This serves two purposes: - Minor: Due to changes in the navigation system, it is possible that existing configuration files will need to be adjusted in order for menus to look the same (in terms of ordering etc.) - Minor: The documentation article component now supports disabling the semantic rendering using a falsy value in https://github.com/hydephp/develop/pull/1566 - Minor: Changed the default build task message to make it more concise in https://github.com/hydephp/develop/pull/1659 +- Minor: Data collection files are now validated for syntax errors during discovery in https://github.com/hydephp/develop/pull/1732 - The `hasFeature` method on the Hyde facade and HydeKernel now only accepts a Feature enum value instead of a string for its parameter. - Changed how the documentation search is generated, to be an `InMemoryPage` instead of a post-build task. - Media asset files are now copied using the new build task instead of the deprecated `BuildService::transferMediaAssets()` method. @@ -232,3 +233,9 @@ Unfortunately, this means that existing setups may need to be adjusted to work w #### Minor impact - Calling the `DataCollection` methods will no longer create the data collections directory automatically + +#### Issues that may arise + +If you start getting `InvalidArgumentException` when using the `DataCollection` class, it may be due to malformed data collection files. +Starting from this version, we validate the syntax of JSON and YAML files during discovery, to help you catch errors early. +See https://github.com/hydephp/develop/issues/1736 for more information. diff --git a/packages/framework/src/Support/DataCollection.php b/packages/framework/src/Support/DataCollection.php index bc8f534e2e5..85b0e78cfd1 100644 --- a/packages/framework/src/Support/DataCollection.php +++ b/packages/framework/src/Support/DataCollection.php @@ -6,6 +6,7 @@ use stdClass; use Hyde\Facades\Filesystem; +use InvalidArgumentException; use Symfony\Component\Yaml\Yaml; use Hyde\Markdown\Models\FrontMatter; use Hyde\Markdown\Models\MarkdownDocument; @@ -83,9 +84,13 @@ public static function yaml(string $name): static */ public static function json(string $name, bool $asArray = false): static { - return static::discover($name, 'json', function (string $file) use ($asArray): stdClass|array|null { + return static::discover($name, 'json', function (string $file) use ($asArray): stdClass|array { $contents = Filesystem::getContents($file); + if (! json_validate($contents)) { + throw new InvalidArgumentException("Invalid JSON in file: '$file'"); + } + return json_decode($contents, $asArray); }); } diff --git a/packages/framework/tests/Feature/DataCollectionTest.php b/packages/framework/tests/Feature/DataCollectionTest.php index 9889472841e..70587bdd95d 100644 --- a/packages/framework/tests/Feature/DataCollectionTest.php +++ b/packages/framework/tests/Feature/DataCollectionTest.php @@ -56,11 +56,11 @@ public function testJsonCollections() { $this->directory('resources/collections/foo'); $this->file('resources/collections/foo/foo.json', json_encode(['foo' => 'bar'])); - $this->file('resources/collections/foo/bar.json'); + $this->file('resources/collections/foo/bar.json', '{"bar": "baz"}'); $this->assertEquals(new DataCollection([ 'foo/foo.json' => (object) ['foo' => 'bar'], - 'foo/bar.json' => null, + 'foo/bar.json' => (object) ['bar' => 'baz'], ]), DataCollection::json('foo')); } @@ -68,11 +68,11 @@ public function testJsonCollectionsAsArrays() { $this->directory('resources/collections/foo'); $this->file('resources/collections/foo/foo.json', json_encode(['foo' => 'bar'])); - $this->file('resources/collections/foo/bar.json'); + $this->file('resources/collections/foo/bar.json', '{"bar": "baz"}'); $this->assertEquals(new DataCollection([ 'foo/foo.json' => ['foo' => 'bar'], - 'foo/bar.json' => null, + 'foo/bar.json' => ['bar' => 'baz'], ]), DataCollection::json('foo', true)); } diff --git a/packages/framework/tests/Unit/DataCollectionUnitTest.php b/packages/framework/tests/Unit/DataCollectionUnitTest.php index b80b68cbb62..336bb209e12 100644 --- a/packages/framework/tests/Unit/DataCollectionUnitTest.php +++ b/packages/framework/tests/Unit/DataCollectionUnitTest.php @@ -6,6 +6,7 @@ use Hyde\Hyde; use Illuminate\Support\Str; +use InvalidArgumentException; use Hyde\Support\DataCollection; use Hyde\Testing\UnitTestCase; use Illuminate\Filesystem\Filesystem; @@ -242,6 +243,67 @@ public function testJsonMethodReturnsCollectionOfJsonDecodedArrays() ], MockableDataCollection::json('foo', true), true); } + public function testJsonMethodThrowsExceptionForInvalidJson() + { + MockableDataCollection::mockFiles([ + 'foo/bar.json' => '{"foo": "bar"', + ]); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Invalid JSON in file: 'foo/bar.json'"); + + MockableDataCollection::json('foo'); + } + + public function testJsonMethodThrowsExceptionForInvalidJsonWithArray() + { + MockableDataCollection::mockFiles([ + 'foo/bar.json' => '{"foo": "bar"', + ]); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Invalid JSON in file: 'foo/bar.json'"); + + MockableDataCollection::json('foo', true); + } + + public function testJsonMethodThrowsExceptionForEmptyJson() + { + MockableDataCollection::mockFiles([ + 'foo/bar.json' => '', + ]); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Invalid JSON in file: 'foo/bar.json'"); + + MockableDataCollection::json('foo'); + } + + public function testJsonMethodThrowsExceptionForBlankJson() + { + MockableDataCollection::mockFiles([ + 'foo/bar.json' => ' ', + ]); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Invalid JSON in file: 'foo/bar.json'"); + + MockableDataCollection::json('foo'); + } + + public function testJsonMethodThrowsExceptionWhenJustOneFileIsInvalid() + { + MockableDataCollection::mockFiles([ + 'foo/bar.json' => '{"foo": "bar"}', + 'foo/baz.json' => '{"foo": "baz"', + ]); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage("Invalid JSON in file: 'foo/baz.json'"); + + MockableDataCollection::json('foo'); + } + protected function assertFrontMatterCollectionStructure(array $expected, DataCollection $collection): void { $this->assertContainsOnlyInstancesOf(FrontMatter::class, $collection);