Skip to content
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

refactor(param-value-converter-registry): wider date converters input type and throw exception #287

Merged
merged 2 commits into from
Jan 30, 2025
Merged
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
55 changes: 40 additions & 15 deletions src/Param/ParamValueConverterRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use DateTimeInterface;
use Psr\Http\Message\StreamInterface;
use SimPod\ClickHouseClient\Exception\UnsupportedParamType;
use SimPod\ClickHouseClient\Exception\UnsupportedParamValue;
use SimPod\ClickHouseClient\Sql\Escaper;
use SimPod\ClickHouseClient\Sql\Type;

Expand All @@ -18,6 +19,8 @@
use function implode;
use function in_array;
use function is_array;
use function is_float;
use function is_int;
use function is_string;
use function json_encode;
use function sprintf;
Expand All @@ -26,13 +29,12 @@
use function trim;

/**
* @phpstan-type Converter = Closure(mixed, Type|string|null, bool):(StreamInterface|string)
* @phpstan-type ConverterRegistry = array<string, Converter>
* @phpstan-type Converter Closure(mixed, Type|string|null, bool):(StreamInterface|string)
* @phpstan-type ConverterRegistry array<string, Converter>
*/
final class ParamValueConverterRegistry
{
/** @var list<string> */
private static array $caseInsensitiveTypes = [
private const CaseInsensitiveTypes = [
'bool',
'date',
'date32',
Expand Down Expand Up @@ -94,9 +96,17 @@
'date32' => self::dateConverter(),
'datetime' => self::dateTimeConverter(),
'datetime32' => self::dateTimeConverter(),
'datetime64' => static fn (DateTimeInterface|string|int|float $value) => $value instanceof DateTimeInterface
? $value->format('U.u')
: $value,
'datetime64' => static function (mixed $value) {
if ($value instanceof DateTimeInterface) {
return $value->format('U.u');
}

if (is_string($value) || is_float($value) || is_int($value)) {
return $value;
}

throw UnsupportedParamValue::type($value);

Check warning on line 108 in src/Param/ParamValueConverterRegistry.php

View check run for this annotation

Codecov / codecov/patch

src/Param/ParamValueConverterRegistry.php#L108

Added line #L108 was not covered by tests
},

'Dynamic' => self::noopConverter(),
'Variant' => self::noopConverter(),
Expand Down Expand Up @@ -232,7 +242,7 @@

$typeName = strtolower($typeName);
$converter = $this->registry[$typeName] ?? null;
if ($converter !== null && in_array($typeName, self::$caseInsensitiveTypes, true)) {
if ($converter !== null && in_array($typeName, self::CaseInsensitiveTypes, true)) {
return $converter;
}

Expand Down Expand Up @@ -271,17 +281,32 @@

private static function dateConverter(): Closure
{
return static fn (DateTimeInterface|string|int|float $value) => $value instanceof DateTimeInterface
// We cannot convert to timestamp yet https://github.com/ClickHouse/ClickHouse/issues/75217
? $value->format('Y-m-d')
: $value;
return static function (mixed $value) {
if ($value instanceof DateTimeInterface) {
return $value->format('Y-m-d');
}

if (is_string($value) || is_float($value) || is_int($value)) {
return $value;
}

throw UnsupportedParamValue::type($value);

Check warning on line 293 in src/Param/ParamValueConverterRegistry.php

View check run for this annotation

Codecov / codecov/patch

src/Param/ParamValueConverterRegistry.php#L293

Added line #L293 was not covered by tests
};
}

private static function dateTimeConverter(): Closure
{
return static fn (DateTimeInterface|string|int|float $value) => $value instanceof DateTimeInterface
? $value->getTimestamp()
: $value;
return static function (mixed $value) {
if ($value instanceof DateTimeInterface) {
return $value->getTimestamp();
}

if (is_string($value) || is_float($value) || is_int($value)) {
return $value;
}

throw UnsupportedParamValue::type($value);

Check warning on line 308 in src/Param/ParamValueConverterRegistry.php

View check run for this annotation

Codecov / codecov/patch

src/Param/ParamValueConverterRegistry.php#L308

Added line #L308 was not covered by tests
};
}

private static function dateIntervalConverter(): Closure
Expand Down
8 changes: 7 additions & 1 deletion tests/Param/ParamValueConverterRegistryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,13 @@ public static function providerConvert(): Generator
yield 'Date' => ['Date', '2023-02-01', '2023-02-01'];
yield 'Date (datetime)' => ['Date', new DateTimeImmutable('2023-02-01'), '2023-02-01'];
yield 'Date32' => ['Date32', new DateTimeImmutable('2023-02-01'), '2023-02-01'];
yield 'DateTime' => ['DateTime', new DateTimeImmutable('2023-02-01 01:02:03'), '2023-02-01 01:02:03'];
yield 'DateTime (string)' => ['DateTime', '2023-02-01 01:02:03', '2023-02-01 01:02:03'];
yield 'DateTime (datetime)' => [
'DateTime',
new DateTimeImmutable('2023-02-01 01:02:03'),
'2023-02-01 01:02:03',
];

yield 'DateTime32' => ['DateTime32', new DateTimeImmutable('2023-02-01 01:02:03'), '2023-02-01 01:02:03'];
yield 'DateTime64(3)' => [
'DateTime64(3)',
Expand Down
Loading