Skip to content

Commit

Permalink
Merge branch '5.1' into 5.2
Browse files Browse the repository at this point in the history
  • Loading branch information
lcharette committed Jun 30, 2024
2 parents d2c9841 + 9842ebb commit 4e26145
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 8 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
24 changes: 16 additions & 8 deletions src/Config/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand All @@ -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.");
Expand All @@ -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.");
Expand All @@ -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.");
Expand Down
25 changes: 25 additions & 0 deletions tests/Config/ConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class ConfigTest extends TestCase
'string' => 'foobar',
'int' => 92,
'array' => ['foo', 'bar'],
'null' => null,
];

public function testGetBool(): void
Expand All @@ -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');
}
Expand All @@ -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');
}
Expand All @@ -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');
}
Expand All @@ -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');
}
Expand Down

0 comments on commit 4e26145

Please sign in to comment.