Time type complete support #9967

A time in DB uses the `TIME` native MariaDB type, with a precision to
the seconds.

However, the GraphQL time type is meant to be more human friendly and is
only precise to the minute. It also accepts a few formats as input. That
should allow the client code to directly forward whatever the human is
typing.

src/Api/Scalar/TimeType.php

@@ -14,15 +14,15 @@
 
 final class TimeType extends ScalarType
 {
-    public ?string $description = 'A time of the day (local time, no timezone).';
+    public ?string $description = 'A time of the day including only hour and minutes (local time, no timezone). Accepted formats are "14h35", "14:35" or "14h".';
 
     /**
      * Serializes an internal value to include in a response.
      */
     public function serialize(mixed $value): mixed
     {
         if ($value instanceof ChronosTime) {
-            return $value->format('H:i:s.u');
+            return $value->format('H\hi');
         }
 
         return $value;
@@ -41,6 +41,11 @@ public function parseValue(mixed $value): ?ChronosTime
             return null;
         }
 
+        if (!preg_match('~^(?<hour>\d{1,2})(([h:]$)|([h:](?<minute>\d{1,2}))?$)~', trim($value), $m)) {
+            throw new UnexpectedValueException('Invalid format  Chronos time. Expected "14h35", "14:35" or "14h", but got: ' . Utils::printSafe($value));
+        }
+
+        $value = $m['hour'] . ':' . ($m['minute'] ?? '00');
         $time = new ChronosTime($value);
 
         return $time;

src/DBAL/Types/TimeType.php

@@ -5,20 +5,44 @@
 namespace Ecodev\Felix\DBAL\Types;
 
 use Cake\Chronos\ChronosTime;
-use DateTimeInterface;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Types\ConversionException;
 
 final class TimeType extends \Doctrine\DBAL\Types\TimeType
 {
     /**
-     * @param null|ChronosTime|DateTimeInterface|string $value
+     * @return ($value is null ? null : string)
+     */
+    public function convertToDatabaseValue(mixed $value, AbstractPlatform $platform): ?string
+    {
+        if ($value === null) {
+            return $value;
+        }
+
+        if ($value instanceof ChronosTime) {
+            return $value->format($platform->getTimeFormatString());
+        }
+
+        throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', 'ChronosTime']);
+    }
+
+    /**
+     * @return ($value is null ? null : ChronosTime)
      */
     public function convertToPHPValue(mixed $value, AbstractPlatform $platform): ?ChronosTime
     {
         if ($value === null || $value instanceof ChronosTime) {
             return $value;
         }
 
+        if (!is_string($value)) {
+            throw ConversionException::conversionFailedFormat(
+                $value,
+                $this->getName(),
+                $platform->getTimeFormatString(),
+            );
+        }
+
         $val = new ChronosTime($value);
 
         return $val;

tests/Api/Scalar/ChronosTypeTest.php

@@ -34,7 +34,7 @@ public function testSerialize(): void
     }
 
     /**
-     * @dataProvider providerValue
+     * @dataProvider providerValues
      */
     public function testParseValue(string $input, ?string $expected): void
     {
@@ -48,7 +48,7 @@ public function testParseValue(string $input, ?string $expected): void
     }
 
     /**
-     * @dataProvider providerValue
+     * @dataProvider providerValues
      */
     public function testParseLiteral(string $input, ?string $expected): void
     {
@@ -72,7 +72,7 @@ public function testParseLiteralAsInt(): void
         $type->parseLiteral($ast);
     }
 
-    public static function providerValue(): array
+    public static function providerValues(): array
     {
         return [
             'UTC' => ['2018-09-14T22:00:00.000Z', '2018-09-15T00:00:00+02:00'],

tests/Api/Scalar/TimeTypeTest.php

@@ -17,12 +17,26 @@ public function testSerialize(): void
         $type = new TimeType();
         $time = new ChronosTime('14:30:25');
         $actual = $type->serialize($time);
-        self::assertSame('14:30:25.000000', $actual);
+        self::assertSame('14h30', $actual);
 
         // Test serialize with microseconds
         $time = new ChronosTime('23:59:59.1254');
         $actual = $type->serialize($time);
-        self::assertSame('23:59:59.001254', $actual);
+        self::assertSame('23h59', $actual);
+    }
+
+    /**
+     * @dataProvider providerValues
+     */
+    public function testParseValue(string $input, ?string $expected): void
+    {
+        $type = new TimeType();
+        $actual = $type->parseValue($input);
+        if ($actual) {
+            $actual = $actual->__toString();
+        }
+
+        self::assertSame($expected, $actual);
     }
 
     /**
@@ -34,8 +48,11 @@ public function testParseLiteral(string $input, ?string $expected): void
         $ast = new StringValueNode(['value' => $input]);
 
         $actual = $type->parseLiteral($ast);
-        self::assertInstanceOf(ChronosTime::class, $actual);
-        self::assertSame($expected, $actual->format('H:i:s.u'));
+        if ($actual) {
+            $actual = $actual->__toString();
+        }
+
+        self::assertSame($expected, $actual);
     }
 
     public function testParseLiteralAsInt(): void
@@ -50,8 +67,14 @@ public function testParseLiteralAsInt(): void
     public static function providerValues(): array
     {
         return [
-            'normal timr' => ['14:30:25', '14:30:25.000000'],
-            'time with milliseconds' => ['23:45:13.300', '23:45:13.000300'],
+            'empty string' => ['', null],
+            'normal time' => ['14:30', '14:30:00'],
+            'alternative separator' => ['14h30', '14:30:00'],
+            'only hour' => ['14h', '14:00:00'],
+            'only hour alternative' => ['14:', '14:00:00'],
+            'even shorter' => ['9', '09:00:00'],
+            'spaces are fines' => ['  14h00  ', '14:00:00'],
+            'a bit weird, but why not' => ['  14h6  ', '14:06:00'],
         ];
     }
 }

tests/DBAL/Types/TimeTypeTest.php

@@ -0,0 +1,63 @@
+<?php
+
+declare(strict_types=1);
+
+namespace EcodevTests\Felix\DBAL\Types;
+
+use Cake\Chronos\ChronosTime;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Platforms\MySQLPlatform;
+use Doctrine\DBAL\Types\ConversionException;
+use Ecodev\Felix\DBAL\Types\TimeType;
+use PHPUnit\Framework\TestCase;
+
+class TimeTypeTest extends TestCase
+{
+    private TimeType $type;
+
+    private AbstractPlatform $platform;
+
+    protected function setUp(): void
+    {
+        $this->type = new TimeType();
+        $this->platform = new MySQLPlatform();
+    }
+
+    public function testConvertToDatabaseValue(): void
+    {
+        self::assertSame('TIME', $this->type->getSqlDeclaration(['foo'], $this->platform));
+        self::assertFalse($this->type->requiresSQLCommentHint($this->platform));
+
+        $actual = $this->type->convertToDatabaseValue(new ChronosTime('09:33'), $this->platform);
+        self::assertSame('09:33:00', $actual, 'support Chronos');
+
+        self::assertNull($this->type->convertToDatabaseValue(null, $this->platform), 'support null values');
+    }
+
+    public function testConvertToPHPValue(): void
+    {
+        $actualPhp = $this->type->convertToPHPValue('18:59:23', $this->platform);
+        self::assertInstanceOf(ChronosTime::class, $actualPhp);
+        self::assertSame('18:59:23', $actualPhp->__toString(), 'support string');
+
+        $actualPhp = $this->type->convertToPHPValue(new ChronosTime('18:59:23'), $this->platform);
+        self::assertInstanceOf(ChronosTime::class, $actualPhp);
+        self::assertSame('18:59:23', $actualPhp->__toString(), 'support ChronosTime');
+
+        self::assertNull($this->type->convertToPHPValue(null, $this->platform), 'support null values');
+    }
+
+    public function testConvertToPHPValueThrowsWithInvalidValue(): void
+    {
+        $this->expectException(ConversionException::class);
+
+        $this->type->convertToPHPValue(123, $this->platform);
+    }
+
+    public function testConvertToDatabaseValueThrowsWithInvalidValue(): void
+    {
+        $this->expectException(ConversionException::class);
+
+        $this->type->convertToDatabaseValue(123, $this->platform);
+    }
+}