diff --git a/CHANGELOG.md b/CHANGELOG.md index 434f037..2d8b00e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [4.3.1] +- Improved code quality & type hinting +- Fixed issue where an empty Yaml file would produce a null. Now returns an empty array +- `UserFrosting\Support\DotenvEditor\DotenvEditor::load` will throw an error if a `null` path is passed. +- Replaced deprecated code in `UserFrosting\Support\Repository\Repository` + ## [4.3.0] - Dropping support for PHP 5.6 & 7.0 - Updated Illuminate/Config to 5.8 @@ -34,6 +40,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a ## 4.0.0 - Initial Release +[4.3.1]: https://github.com/userfrosting/support/compare/4.3.0...4.3.1 [4.3.0]: https://github.com/userfrosting/support/compare/4.2.1...4.3.0 [4.2.1]: https://github.com/userfrosting/support/compare/4.2.0...4.2.1 [4.2.0]: https://github.com/userfrosting/support/compare/4.1.3...4.2.0 diff --git a/composer.json b/composer.json index 99377eb..42eb4bb 100644 --- a/composer.json +++ b/composer.json @@ -26,5 +26,10 @@ "psr-4": { "UserFrosting\\Support\\": "src" } + }, + "autoload-dev": { + "psr-4": { + "UserFrosting\\Support\\Tests\\": "tests" + } } } diff --git a/src/DotenvEditor/DotenvEditor.php b/src/DotenvEditor/DotenvEditor.php index d05c0bd..f682e8c 100644 --- a/src/DotenvEditor/DotenvEditor.php +++ b/src/DotenvEditor/DotenvEditor.php @@ -50,7 +50,7 @@ public function __construct($backupPath = '', $autoBackup = true) /** * Load file for working. * - * @param string|null $filePath The file path + * @param string $filePath The file path * @param bool $restoreIfNotFound Restore this file from other file if it's not found * @param string|null $restorePath The file path you want to restore from * @@ -58,6 +58,11 @@ public function __construct($backupPath = '', $autoBackup = true) */ public function load($filePath = null, $restoreIfNotFound = false, $restorePath = null) { + //Fail if path is null to maintain compatibility with Jackiedo\DotenvEditor + if (is_null($filePath)) { + throw new \InvalidArgumentException('File path cannot be null'); + } + $this->resetContent(); $this->filePath = $filePath; @@ -68,7 +73,9 @@ public function load($filePath = null, $restoreIfNotFound = false, $restorePath return $this; } elseif ($restoreIfNotFound) { - return $this->restore($restorePath); + $this->restore($restorePath); + + return $this; } else { return $this; } diff --git a/src/Repository/Loader/ArrayFileLoader.php b/src/Repository/Loader/ArrayFileLoader.php index 2723333..98c0657 100644 --- a/src/Repository/Loader/ArrayFileLoader.php +++ b/src/Repository/Loader/ArrayFileLoader.php @@ -18,9 +18,9 @@ class ArrayFileLoader extends FileRepositoryLoader { /** - * @return array + * {@inheritdoc} */ - protected function parseFile($path) + protected function parseFile(string $path): array { return require $path; } diff --git a/src/Repository/Loader/FileRepositoryLoader.php b/src/Repository/Loader/FileRepositoryLoader.php index e7d86fb..f65c799 100644 --- a/src/Repository/Loader/FileRepositoryLoader.php +++ b/src/Repository/Loader/FileRepositoryLoader.php @@ -20,7 +20,7 @@ abstract class FileRepositoryLoader { /** - * @var array An array of paths to ultimately load the data from. + * @var string[] An array of paths to ultimately load the data from. */ protected $paths = []; @@ -39,18 +39,18 @@ public function __construct($paths) * * @param string $path * - * @return array + * @return mixed[] */ - abstract protected function parseFile($path); + abstract protected function parseFile(string $path): array; /** * Fetch and recursively merge in content from all file paths. * * @param bool $skipMissing * - * @return array + * @return string[] */ - public function load($skipMissing = true) + public function load(bool $skipMissing = true): array { $result = []; @@ -70,9 +70,9 @@ public function load($skipMissing = true) * * @throws FileNotFoundException * - * @return array + * @return mixed[] */ - public function loadFile($path, $skipMissing = true) + public function loadFile(string $path, $skipMissing = true): array { if (!file_exists($path)) { if ($skipMissing) { @@ -97,7 +97,7 @@ public function loadFile($path, $skipMissing = true) * * @return bool */ - protected function isReadable($path) + protected function isReadable(string $path): bool { return is_readable($path); } @@ -107,7 +107,7 @@ protected function isReadable($path) * * @param string $path */ - public function addPath($path) + public function addPath(string $path): self { $this->paths[] = rtrim($path, '/\\'); @@ -119,7 +119,7 @@ public function addPath($path) * * @param string $path */ - public function prependPath($path) + public function prependPath($path): self { array_unshift($this->paths, rtrim($path, '/\\')); @@ -131,7 +131,7 @@ public function prependPath($path) * * @param string|string[] $paths */ - public function setPaths($paths) + public function setPaths($paths): self { if (!is_array($paths)) { $paths = [$paths]; @@ -149,9 +149,9 @@ public function setPaths($paths) /** * Return a list of all file paths. * - * @return array + * @return string[] */ - public function getPaths() + public function getPaths(): array { return $this->paths; } diff --git a/src/Repository/Loader/YamlFileLoader.php b/src/Repository/Loader/YamlFileLoader.php index e7ba566..0cc10b3 100644 --- a/src/Repository/Loader/YamlFileLoader.php +++ b/src/Repository/Loader/YamlFileLoader.php @@ -25,7 +25,7 @@ class YamlFileLoader extends FileRepositoryLoader /** * {@inheritdoc} */ - protected function parseFile($path) + protected function parseFile(string $path): array { $doc = $this->fileGetContents($path); if ($doc === false) { @@ -42,6 +42,11 @@ protected function parseFile($path) } } + // In case `Yaml::parse` returns empty data/file + if (is_null($result)) { + return []; + } + return $result; } @@ -50,9 +55,9 @@ protected function parseFile($path) * * @param string $path * - * @return string|bool + * @return string|false */ - protected function fileGetContents($path) + protected function fileGetContents(string $path) { return file_get_contents($path); } diff --git a/src/Repository/PathBuilder/PathBuilder.php b/src/Repository/PathBuilder/PathBuilder.php index 0577e66..bd1b59b 100644 --- a/src/Repository/PathBuilder/PathBuilder.php +++ b/src/Repository/PathBuilder/PathBuilder.php @@ -35,7 +35,7 @@ abstract class PathBuilder * @param ResourceLocatorInterface $locator * @param string $uri */ - public function __construct(ResourceLocatorInterface $locator, $uri) + public function __construct(ResourceLocatorInterface $locator, string $uri) { $this->locator = $locator; $this->uri = $uri; @@ -46,5 +46,5 @@ public function __construct(ResourceLocatorInterface $locator, $uri) * * @return array */ - abstract public function buildPaths(); + abstract public function buildPaths(): array; } diff --git a/src/Repository/PathBuilder/SimpleGlobBuilder.php b/src/Repository/PathBuilder/SimpleGlobBuilder.php index 01b7c8a..d38d3b5 100644 --- a/src/Repository/PathBuilder/SimpleGlobBuilder.php +++ b/src/Repository/PathBuilder/SimpleGlobBuilder.php @@ -20,9 +20,11 @@ class SimpleGlobBuilder extends PathBuilder /** * Glob together all file paths in each search path from the locator. * + * @param string $extension (default 'php') + * * @return array */ - public function buildPaths($extension = 'php') + public function buildPaths(string $extension = 'php'): array { // Get all paths from the locator that match the uri. // Put them in reverse order to allow later files to override earlier files. diff --git a/src/Repository/PathBuilder/StreamPathBuilder.php b/src/Repository/PathBuilder/StreamPathBuilder.php index 83975b7..7130b3a 100644 --- a/src/Repository/PathBuilder/StreamPathBuilder.php +++ b/src/Repository/PathBuilder/StreamPathBuilder.php @@ -22,7 +22,7 @@ class StreamPathBuilder extends PathBuilder * * @return array */ - public function buildPaths() + public function buildPaths(): array { // Get all paths from the locator that match the uri. // Put them in reverse order to allow later files to override earlier files. diff --git a/src/Repository/Repository.php b/src/Repository/Repository.php index 97229fa..fcbaf41 100644 --- a/src/Repository/Repository.php +++ b/src/Repository/Repository.php @@ -11,6 +11,7 @@ namespace UserFrosting\Support\Repository; use Illuminate\Config\Repository as IlluminateRepository; +use Illuminate\Support\Arr; use UserFrosting\Support\Util\Util; /** @@ -32,9 +33,9 @@ class Repository extends IlluminateRepository * @param string|null $key * @param mixed $items */ - public function mergeItems($key, $items) + public function mergeItems(?string $key, $items): self { - $targetValues = array_get($this->items, $key); + $targetValues = Arr::get($this->items, $key); if (is_array($targetValues)) { $modifiedValues = array_replace_recursive($targetValues, $items); @@ -42,7 +43,7 @@ public function mergeItems($key, $items) $modifiedValues = $items; } - array_set($this->items, $key, $modifiedValues); + Arr::set($this->items, $key, $modifiedValues); return $this; } @@ -50,7 +51,7 @@ public function mergeItems($key, $items) /** * Get the specified configuration value, recursively removing all null values. * - * @param string $key + * @param string|array|null $key * * @return mixed */ diff --git a/src/Util/Util.php b/src/Util/Util.php index cfbbde1..c4cbeac 100644 --- a/src/Util/Util.php +++ b/src/Util/Util.php @@ -111,7 +111,7 @@ public static function stripPrefix($str, $prefix = '') * * @param string|array $patterns * @param string $subject - * @param array &$matches + * @param array $matches * @param string $delimiter * @param int $flags * @param int $offset diff --git a/tests/DotenvEditor/DotenvEditorTest.php b/tests/DotenvEditor/DotenvEditorTest.php index 044325f..6f04333 100644 --- a/tests/DotenvEditor/DotenvEditorTest.php +++ b/tests/DotenvEditor/DotenvEditorTest.php @@ -8,6 +8,8 @@ * @license https://github.com/userfrosting/support/blob/master/LICENSE.md (MIT License) */ +namespace UserFrosting\Support\Tests\DotenvEditor; + use PHPUnit\Framework\TestCase; use UserFrosting\Support\DotenvEditor\DotenvEditor; @@ -73,6 +75,16 @@ public function testLoadPathNotExist() $this->assertEquals($editor, $result); } + /** + * @depends testConstructor + */ + public function testLoadPathIsNull() + { + $editor = new DotenvEditor($this->basePath.'.env-backups/'); + $this->expectException(\InvalidArgumentException::class); + $result = $editor->load(); + } + /** * @depends testConstructor */ diff --git a/tests/Exception/HttpExceptionTest.php b/tests/Exception/HttpExceptionTest.php index 527212f..e16c67f 100644 --- a/tests/Exception/HttpExceptionTest.php +++ b/tests/Exception/HttpExceptionTest.php @@ -8,6 +8,8 @@ * @license https://github.com/userfrosting/support/blob/master/LICENSE.md (MIT License) */ +namespace UserFrosting\Support\Tests\Exception; + use PHPUnit\Framework\TestCase; use UserFrosting\Support\Exception\HttpException; use UserFrosting\Support\Message\UserMessage; diff --git a/tests/Repository/PathBuilderTest.php b/tests/Repository/PathBuilderTest.php index 020c04c..69e1344 100644 --- a/tests/Repository/PathBuilderTest.php +++ b/tests/Repository/PathBuilderTest.php @@ -8,6 +8,8 @@ * @license https://github.com/userfrosting/support/blob/master/LICENSE.md (MIT License) */ +namespace UserFrosting\Support\Tests\Repository; + use PHPUnit\Framework\TestCase; use UserFrosting\Support\Repository\PathBuilder\SimpleGlobBuilder; use UserFrosting\Support\Repository\PathBuilder\StreamPathBuilder; diff --git a/tests/Repository/RepositoryLoaderTest.php b/tests/Repository/RepositoryLoaderTest.php index 66b9bee..2f91cbb 100644 --- a/tests/Repository/RepositoryLoaderTest.php +++ b/tests/Repository/RepositoryLoaderTest.php @@ -8,6 +8,8 @@ * @license https://github.com/userfrosting/support/blob/master/LICENSE.md (MIT License) */ +namespace UserFrosting\Support\Tests\Repository; + use PHPUnit\Framework\TestCase; use UserFrosting\Support\Exception\FileNotFoundException; use UserFrosting\Support\Exception\JsonException; @@ -188,6 +190,62 @@ public function testYamlFileLoaderWithFalseFileContent() $data = $loader->load(); } + /** + * Make sure an empty file doesn't mess up by returning null. + * + * @depends testYamlFileLoader + * @depends testYamlFileLoaderWithStringPath + */ + public function testYamlFileLoaderWithNoFileContent() + { + $loader = new YamlFileLoader(__DIR__.'/data/core/owls/empty.yaml'); + + // Act + $data = $loader->load(); + + $this->assertEquals([], $data); + } + + /** + * Make sure an empty file doesn't mess up by returning null. + * + * @depends testYamlFileLoaderWithNoFileContent + */ + public function testYamlFileLoaderWithNoFileContentOnFirstFile() + { + $loader = new YamlFileLoader([ + __DIR__.'/data/core/owls/empty.yaml', + __DIR__.'/data/core/owls/tyto.yaml', + ]); + + // Act + $data = $loader->load(); + + $this->assertEquals([ + 'plumage' => 'floofy', + ], $data); + } + + /** + * Make sure an empty file doesn't mess up by returning null. + * + * @depends testYamlFileLoaderWithNoFileContent + */ + public function testYamlFileLoaderWithNoFileContentOnSecondFile() + { + $loader = new YamlFileLoader([ + __DIR__.'/data/core/owls/tyto.yaml', + __DIR__.'/data/core/owls/empty.yaml', + ]); + + // Act + $data = $loader->load(); + + $this->assertEquals([ + 'plumage' => 'floofy', + ], $data); + } + /** * @depends testYamlFileLoader */ diff --git a/tests/Repository/RepositoryTest.php b/tests/Repository/RepositoryTest.php index 559887b..12d74cf 100644 --- a/tests/Repository/RepositoryTest.php +++ b/tests/Repository/RepositoryTest.php @@ -8,6 +8,8 @@ * @license https://github.com/userfrosting/support/blob/master/LICENSE.md (MIT License) */ +namespace UserFrosting\Support\Tests\Repository; + use PHPUnit\Framework\TestCase; use UserFrosting\Support\Repository\Repository; @@ -65,6 +67,23 @@ public function testGetDefinedWithString() $this->assertEquals(4, $defined); } + /** + * @depends testGetDefined + */ + public function testGetDefinedWithArray() + { + $repo = new Repository($this->data); + + $defined = $repo->getDefined(['chicks', 'voles']); + + $this->assertEquals([ + 'chicks' => 4, + 'voles' => [ + 'caught' => 8, + ], + ], $defined); + } + /** * @depends testGetDefinedWithString */ @@ -95,4 +114,21 @@ public function testMergeItemsWithArray() $this->assertEquals($newData, $defined); } + + /** + * @depends testGetDefinedWithString + */ + public function testMergeItemsWithNull() + { + $repo = new Repository($this->data); + + $newData = [ + 'foo' => 'bar', + ]; + + $repo->mergeItems(null, $newData); + $defined = $repo->getDefined('foo'); + + $this->assertEquals('bar', $defined); + } } diff --git a/tests/Repository/data/core/owls/empty.yaml b/tests/Repository/data/core/owls/empty.yaml new file mode 100644 index 0000000..e69de29 diff --git a/tests/Util/UtilTest.php b/tests/Util/UtilTest.php index ef8e275..3d83d57 100644 --- a/tests/Util/UtilTest.php +++ b/tests/Util/UtilTest.php @@ -8,6 +8,8 @@ * @license https://github.com/userfrosting/support/blob/master/LICENSE.md (MIT License) */ +namespace UserFrosting\Support\Tests\Util; + use PHPUnit\Framework\TestCase; use UserFrosting\Support\Util\Util;