diff --git a/CHANGELOG.md b/CHANGELOG.md index 34af800..c80be37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a ## [5.2.0](https://github.com/userfrosting/framework/compare/5.0.1...5.2.0) +## [5.1.3](https://github.com/userfrosting/framework/compare/5.1.2...5.1.3) +- [Config] Fix issue with `getBool`, `getString`, `getInt` and `getArray` where a null value could be returned even if a default parameter was provided when the data did in fact return `null`, making the return value not type safe as it should be. + ## [5.1.2](https://github.com/userfrosting/framework/compare/5.1.1...5.1.2) - Add Slim [type hinting](https://github.com/slimphp/Slim/releases/tag/4.14.0) & bump minimum slim version diff --git a/src/Config/Config.php b/src/Config/Config.php index 7405a86..12f6152 100644 --- a/src/Config/Config.php +++ b/src/Config/Config.php @@ -21,15 +21,17 @@ class Config extends Repository { /** * Get the specified configuration value as bool. + * Returns null if the key is not found, unless a default is provided, in + * which case the return value is type safe (can't be null). * * @param string $key * @param bool|null $default * - * @return ($default is null ? bool|null : bool) Returns null if the key is not found + * @return ($default is null ? bool|null : bool) */ public function getBool(string $key, ?bool $default = null): ?bool { - $value = $this->get($key, $default); + $value = $this->get($key, $default) ?? $default; if (!is_bool($value) && !is_null($value)) { throw new TypeException("Config key '$key' doesn't return bool."); @@ -40,15 +42,17 @@ public function getBool(string $key, ?bool $default = null): ?bool /** * Get the specified configuration value as bool. + * Returns null if the key is not found, unless a default is provided, in + * which case the return value is type safe (can't be null). * * @param string $key * @param string|null $default * - * @return ($default is null ? string|null : string) Returns null if the key is not found + * @return ($default is null ? string|null : string) */ public function getString(string $key, ?string $default = null): ?string { - $value = $this->get($key, $default); + $value = $this->get($key, $default) ?? $default; if (!is_string($value) && !is_null($value)) { throw new TypeException("Config key '$key' doesn't return string."); @@ -59,15 +63,17 @@ public function getString(string $key, ?string $default = null): ?string /** * Get the specified configuration value as bool. + * Returns null if the key is not found, unless a default is provided, in + * which case the return value is type safe (can't be null). * * @param string $key * @param int|null $default * - * @return ($default is null ? int|null : int) Returns null if the key is not found + * @return ($default is null ? int|null : int) */ public function getInt(string $key, ?int $default = null): ?int { - $value = $this->get($key, $default); + $value = $this->get($key, $default) ?? $default; if (!is_int($value) && !is_null($value)) { throw new TypeException("Config key '$key' doesn't return int."); @@ -78,15 +84,17 @@ public function getInt(string $key, ?int $default = null): ?int /** * Get the specified configuration value as bool. + * Returns null if the key is not found, unless a default is provided, in + * which case the return value is type safe (can't be null). * * @param string $key * @param mixed[]|null $default * - * @return ($default is null ? mixed[]|null : mixed[]) Returns null if the key is not found + * @return ($default is null ? mixed[]|null : mixed[]) */ public function getArray(string $key, ?array $default = null): ?array { - $value = $this->get($key, $default); + $value = $this->get($key, $default) ?? $default; if (!is_array($value) && !is_null($value)) { throw new TypeException("Config key '$key' doesn't return array."); diff --git a/tests/Config/ConfigTest.php b/tests/Config/ConfigTest.php index 1f29d5a..fa6acb1 100644 --- a/tests/Config/ConfigTest.php +++ b/tests/Config/ConfigTest.php @@ -20,6 +20,7 @@ class ConfigTest extends TestCase 'string' => 'foobar', 'int' => 92, 'array' => ['foo', 'bar'], + 'null' => null, ]; public function testGetBool(): void @@ -30,6 +31,12 @@ public function testGetBool(): void $this->assertSame(false, $repo->getBool('missing', false)); $this->assertSame(null, $repo->getBool('missing')); $this->assertSame($repo->get('missing'), $repo->getBool('missing')); // Same default behavior as "get" + + // Value is null, but not the default. Default should still be used. + $this->assertSame(null, $repo->getBool('null')); + $this->assertSame(false, $repo->getBool('null', false)); + + // Exception $this->expectException(TypeException::class); $repo->getBool('string'); } @@ -42,6 +49,12 @@ public function testGetString(): void $this->assertSame('barfoo', $repo->getString('missing', 'barfoo')); $this->assertSame(null, $repo->getString('missing')); $this->assertSame($repo->get('missing'), $repo->getString('missing')); // Same default behavior as "get" + + // Value is null, but not the default. Default should still be used. + $this->assertSame(null, $repo->getString('null')); + $this->assertSame('non-null', $repo->getString('null', 'non-null')); + + // Exception $this->expectException(TypeException::class); $repo->getString('bool'); } @@ -54,6 +67,12 @@ public function testGetInt(): void $this->assertSame(29, $repo->getInt('missing', 29)); $this->assertSame(null, $repo->getInt('missing')); $this->assertSame($repo->get('missing'), $repo->getInt('missing')); // Same default behavior as "get" + + // Value is null, but not the default. Default should still be used. + $this->assertSame(null, $repo->getInt('null')); + $this->assertSame(123, $repo->getInt('null', 123)); + + // Exception $this->expectException(TypeException::class); $repo->getInt('string'); } @@ -66,6 +85,12 @@ public function testGetArray(): void $this->assertSame(['bar', 'foo'], $repo->getArray('missing', ['bar', 'foo'])); $this->assertSame(null, $repo->getArray('missing')); $this->assertSame($repo->get('missing'), $repo->getArray('missing')); // Same default behavior as "get" + + // Value is null, but not the default. Default should still be used. + $this->assertSame(null, $repo->getArray('null')); + $this->assertSame(['non-null'], $repo->getArray('null', ['non-null'])); + + // Exception $this->expectException(TypeException::class); $repo->getArray('string'); }