Skip to content

feat: Improve Superglobals service #9482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: 4.7
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions system/HTTP/Parameters/InputParameters.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

declare(strict_types=1);

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <[email protected]>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\HTTP\Parameters;

use CodeIgniter\Exceptions\InvalidArgumentException;
use CodeIgniter\Exceptions\RuntimeException;

/**
* @template TKey of string
* @template TValue of scalar|array<(int|string), mixed>
*
* @extends Parameters<TKey, TValue>
*
* @see \CodeIgniter\HTTP\Parameters\InputParametersTest
*/
class InputParameters extends Parameters
{
public function override(array $parameters = []): void
{
$this->parameters = [];

foreach ($parameters as $key => $value) {
$this->set($key, $value);
}
}

public function get(string $key, mixed $default = null): mixed
{
if ($default !== null && ! is_scalar($default)) {
throw new InvalidArgumentException(sprintf('The default value for the InputParameters must be a scalar type, "%s" given.', gettype($default)));
}

// TODO: We need to check that the default value is set. Let's check the unique string
$tempDefault = bin2hex(random_bytes(8));

$value = parent::get($key, $tempDefault);

if ($value !== null && $value !== $tempDefault && ! is_scalar($value)) {
throw new RuntimeException(sprintf('The value of the key "%s" InputParameters does not contain a scalar value, "%s" given.', $key, gettype($value)));
}

return $value === $tempDefault ? $default : $value;
}

public function set(string $key, mixed $value): void
{
if (! is_scalar($value) && ! is_array($value)) {
throw new InvalidArgumentException(sprintf('The value for the InputParameters must be a scalar type, "%s" given.', gettype($value)));
}

$this->parameters[$key] = $value;
}
}
89 changes: 89 additions & 0 deletions system/HTTP/Parameters/Parameters.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php

declare(strict_types=1);

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <[email protected]>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\HTTP\Parameters;

use ArrayIterator;
use CodeIgniter\Exceptions\RuntimeException;

/**
* @template TKey of string
* @template TValue
*
* @implements ParametersInterface<TKey, TValue>
*
* @see \CodeIgniter\HTTP\Parameters\ParametersTest
*/
class Parameters implements ParametersInterface
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this class should be abstract by itself, unless there's a superglobal that is being represented by this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will not be a global object. It might be worth making it abstract. Let's wait for the other participants.
But I wrote general tests for it.

{
/**
* @param array<TKey, TValue> $parameters
*/
public function __construct(
protected array $parameters = [],
) {
}

public function override(array $parameters = []): void
{
$this->parameters = $parameters;
}

public function has(string $key): bool
{
return array_key_exists($key, $this->parameters);
}

public function get(string $key, mixed $default = null): mixed
{
return array_key_exists($key, $this->parameters) ? $this->parameters[$key] : $default;
}

public function set(string $key, mixed $value): void
{
$this->parameters[$key] = $value;
}

public function remove(string $key): void
{
unset($this->parameters[$key]);
}

public function all(?string $key = null): array
{
if ($key === null) {
return $this->parameters;
}

if (! isset($this->parameters[$key]) || ! is_array($this->parameters[$key])) {
throw new RuntimeException(sprintf('The key "%s" value for Parameters is not an array or was not found.', $key));
}

return $this->parameters[$key];
}

public function keys(): array
{
return array_keys($this->parameters);
}

public function getIterator(): ArrayIterator
{
return new ArrayIterator($this->parameters);
}

public function count(): int
{
return count($this->parameters);
}
}
67 changes: 67 additions & 0 deletions system/HTTP/Parameters/ParametersInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?php

declare(strict_types=1);

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <[email protected]>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\HTTP\Parameters;

use Countable;
use IteratorAggregate;

/**
* @template TKey of string
* @template TValue
*
* @extends IteratorAggregate<TKey, TValue>
*/
interface ParametersInterface extends IteratorAggregate, Countable
{
/**
* @param array<TKey, TValue> $parameters
*/
public function __construct(array $parameters = []);

/**
* @param array<TKey, TValue> $parameters
*/
public function override(array $parameters = []): void;

/**
* @param TKey $key
*/
public function has(string $key): bool;

/**
* @param TKey $key
* @param TValue $default

Check failure on line 44 in system/HTTP/Parameters/ParametersInterface.php

View workflow job for this annotation

GitHub Actions / Psalm Analysis

InvalidParamDefault

system/HTTP/Parameters/ParametersInterface.php:44:15: InvalidParamDefault: Default value type null for argument 2 of method CodeIgniter\HTTP\Parameters\InputParameters::get does not match the given type TValue:CodeIgniter\HTTP\Parameters\InputParameters as array<int|string, mixed>|scalar (see https://psalm.dev/062)
*
* @return TValue|null
*/
Comment on lines +43 to +47
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand now the issue. You need to specify another template for this default. Like:

Suggested change
* @param TKey $key
* @param TValue $default
*
* @return TValue|null
*/
* @template TDefault
*
* @param TKey $key
* @param TDefault $default
*
* @return TValue|TDefault|null
*/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the problem.
I didn't plan on null in scalar data. $_GET['a'] = null from the request will never exist. Yes, this is acceptable in other places.

Are there any examples for null tests?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the tinkering I made to allow strict default value and not allowing null for InputParameters.

diff --git a/system/HTTP/Parameters/InputParameters.php b/system/HTTP/Parameters/InputParameters.php
index 8aa0508075..a45f7b22bd 100644
--- a/system/HTTP/Parameters/InputParameters.php
+++ b/system/HTTP/Parameters/InputParameters.php
@@ -19,8 +19,9 @@ use CodeIgniter\Exceptions\RuntimeException;
 /**
  * @template TKey of string
  * @template TValue of scalar|array<(int|string), mixed>
+ * @template TDefault of scalar|array<(int|string), mixed>
  *
- * @extends Parameters<TKey, TValue>
+ * @extends Parameters<TKey, TValue, TDefault>
  *
  * @see \CodeIgniter\HTTP\Parameters\InputParametersTest
  */
@@ -35,7 +36,7 @@ class InputParameters extends Parameters
         }
     }
 
-    public function get(string $key, mixed $default = null): mixed
+    public function get(string $key, mixed $default = ''): mixed
     {
         if ($default !== null && ! is_scalar($default)) {
             throw new InvalidArgumentException(sprintf('The default value for the InputParameters must be a scalar type, "%s" given.', gettype($defau
lt)));
diff --git a/system/HTTP/Parameters/Parameters.php b/system/HTTP/Parameters/Parameters.php
index 20f9b7d66c..8629f750d2 100644
--- a/system/HTTP/Parameters/Parameters.php
+++ b/system/HTTP/Parameters/Parameters.php
@@ -19,8 +19,9 @@ use CodeIgniter\Exceptions\RuntimeException;
 /**
  * @template TKey of string
  * @template TValue
+ * @template TDefault
  *
- * @implements ParametersInterface<TKey, TValue>
+ * @implements ParametersInterface<TKey, TValue, TDefault>
  *
  * @see \CodeIgniter\HTTP\Parameters\ParametersTest
  */
diff --git a/system/HTTP/Parameters/ParametersInterface.php b/system/HTTP/Parameters/ParametersInterface.php
index 6873e78f78..21ccd09744 100644
--- a/system/HTTP/Parameters/ParametersInterface.php
+++ b/system/HTTP/Parameters/ParametersInterface.php
@@ -19,6 +19,7 @@ use IteratorAggregate;
 /**
  * @template TKey of string
  * @template TValue
+ * @template TDefault
  *
  * @extends IteratorAggregate<TKey, TValue>
  */
@@ -41,9 +42,9 @@ interface ParametersInterface extends IteratorAggregate, Countable
 
     /**
      * @param TKey   $key
-     * @param TValue $default
+     * @param TDefault $default
      *
-     * @return TValue|null
+     * @return TValue|TDefault|null
      */
     public function get(string $key, mixed $default = null): mixed;
 
diff --git a/system/HTTP/Parameters/ServerParameters.php b/system/HTTP/Parameters/ServerParameters.php
index c6e87d49ae..0bbf8f2b07 100644
--- a/system/HTTP/Parameters/ServerParameters.php
+++ b/system/HTTP/Parameters/ServerParameters.php
@@ -16,8 +16,9 @@ namespace CodeIgniter\HTTP\Parameters;
 /**
  * @template TKey of string
  * @template TValue
+ * @template TDefault
  *
- * @extends Parameters<TKey, TValue>
+ * @extends Parameters<TKey, TValue, TDefault>
  */
 class ServerParameters extends Parameters
 {

public function get(string $key, mixed $default = null): mixed;

/**
* @param TKey $key
* @param TValue $value
*/
public function set(string $key, mixed $value): void;

/**
* @param TKey $key
*
* @return array<TKey, TValue>
*/
public function all(?string $key = null): array;

/**
* @return list<TKey>
*/
public function keys(): array;
}
24 changes: 24 additions & 0 deletions system/HTTP/Parameters/ServerParameters.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

declare(strict_types=1);

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <[email protected]>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\HTTP\Parameters;

/**
* @template TKey of string
* @template TValue
*
* @extends Parameters<TKey, TValue>
*/
class ServerParameters extends Parameters
{
}
107 changes: 107 additions & 0 deletions tests/system/HTTP/Parameters/InputParametersTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
<?php

declare(strict_types=1);

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <[email protected]>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\HTTP\Parameters;

use CodeIgniter\Exceptions\InvalidArgumentException;
use CodeIgniter\Test\CIUnitTestCase;
use PHPUnit\Framework\Attributes\Group;
use stdClass;

/**
* @internal
*/
#[Group('Others')]
final class InputParametersTest extends CIUnitTestCase
{
/**
* @var array<string, mixed>
*/
private array $original = [
'title' => '',
'toolbar' => '1',
'path' => 'public/index.php',
'current_time' => 1741522635.661474,
'debug' => true,
'pages' => 15,
'filters' => [
0 => 'name',
1 => 'sum',
],
'sort' => [
'date' => 'ASC',
'age' => 'DESC',
],
];

public function testGetNonScalarValues(): void
{
$parameters = new InputParameters($this->original);

$this->assertNull($parameters->get('undefined_or_null', null));

$this->expectException(InvalidArgumentException::class);

$parameters->get('undefined_throw', new stdClass()); // @phpstan-ignore argument.type
}

public function testAttemptSetNullValues(): void
{
$parameters = new InputParameters($this->original);

$this->expectException(InvalidArgumentException::class);

$parameters->set('nullable', null); // @phpstan-ignore argument.type
}

public function testAttemptSetNonScalarValues(): void
{
$parameters = new InputParameters($this->original);

$this->expectException(InvalidArgumentException::class);

$parameters->set('nullable', new stdClass()); // @phpstan-ignore argument.type
}

public function testUpdateAndSetNewValues(): void
{
/**
* @var array<string, mixed>
*/
$expected = [
'title' => 'CodeIgniter',
'toolbar' => '0',
'path' => '',
'current_time' => 1741522888.661434,
'debug' => false,
'pages' => 10,
'filters' => [
0 => 'sum',
1 => 'name',
],
'sort' => [
'age' => 'ASC',
'date' => 'DESC',
],
'slug' => 'Ben-i-need-help',
];

$parameters = new InputParameters($this->original);

foreach (array_keys($expected) as $key) {
$parameters->set($key, $expected[$key]);
}

$this->assertSame($expected, $parameters->all());
}
}
Loading
Loading