Merge pull request #570: Fix decoding of non-nullable interval fields…

… when null is gotten
temporalio · Feb 19, 2025 · 347e344 · 347e344
2 parents 2b4202b + 9a9a87a
commit 347e344
Show file tree

Hide file tree

Showing 8 changed files with 107 additions and 37 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="6.4.1@09a200c15910905ddc49e5edd37b73f9c78f7580">
+<files psalm-version="6.7.1@a2f190972555ea01b0cfcc1913924d6c5fc1a64e">
   <file src="src/Activity.php">
     <ImplicitToStringCast>
       <code><![CDATA[$type]]></code>
@@ -51,19 +51,9 @@
     </ArgumentTypeCoercion>
   </file>
   <file src="src/Client/Schedule/Action/StartWorkflowAction.php">
-    <DocblockTypeContradiction>
-      <code><![CDATA[$timeout === null]]></code>
-      <code><![CDATA[$timeout === null]]></code>
-      <code><![CDATA[$timeout === null]]></code>
-    </DocblockTypeContradiction>
     <InaccessibleProperty>
       <code><![CDATA[$workflowType->name]]></code>
     </InaccessibleProperty>
-    <RedundantConditionGivenDocblockType>
-      <code><![CDATA[DateInterval::parse($timeout, DateInterval::FORMAT_SECONDS)]]></code>
-      <code><![CDATA[DateInterval::parse($timeout, DateInterval::FORMAT_SECONDS)]]></code>
-      <code><![CDATA[DateInterval::parse($timeout, DateInterval::FORMAT_SECONDS)]]></code>
-    </RedundantConditionGivenDocblockType>
   </file>
   <file src="src/Client/Schedule/Info/ScheduleActionResult.php">
     <PropertyNotSetInConstructor>
@@ -219,8 +209,6 @@
   </file>
   <file src="src/Common/RetryOptions.php">
     <PossiblyNullArgument>
-      <code><![CDATA[$interval]]></code>
-      <code><![CDATA[$interval]]></code>
       <code><![CDATA[DateInterval::toDuration($this->initialInterval)]]></code>
       <code><![CDATA[DateInterval::toDuration($this->maximumInterval)]]></code>
     </PossiblyNullArgument>
@@ -1141,6 +1129,11 @@
       <code><![CDATA[$header ?? Header::empty()]]></code>
     </PropertyTypeCoercion>
   </file>
+  <file src="src/Internal/Workflow/Process/DeferredGenerator.php">
+    <UnevaluatedCode>
+      <code><![CDATA[yield;]]></code>
+    </UnevaluatedCode>
+  </file>
   <file src="src/Internal/Workflow/Process/Process.php">
     <MissingClosureParamType>
       <code><![CDATA[$result]]></code>

diff --git a/psalm.xml b/psalm.xml
@@ -21,4 +21,8 @@
             <directory name="vendor" />
         </ignoreFiles>
     </projectFiles>
+    <issueHandlers>
+        <MissingOverrideAttribute errorLevel="suppress" />
+        <ClassMustBeFinal errorLevel="suppress" />
+    </issueHandlers>
 </psalm>
diff --git a/src/Internal/Marshaller/Type/DurationJsonType.php b/src/Internal/Marshaller/Type/DurationJsonType.php
@@ -81,7 +81,7 @@ public function parse($value, $current): CarbonInterval
         }
 
         if ($value === null) {
-            return CarbonInterval::create();
+            return CarbonInterval::create(0);
         }
 
         return DateInterval::parse($value, $this->fallbackFormat);

diff --git a/src/Internal/Support/DateInterval.php b/src/Internal/Support/DateInterval.php
@@ -16,7 +16,7 @@
 
 /**
  * @psalm-type DateIntervalFormat = DateInterval::FORMAT_*
- * @psalm-type DateIntervalValue = string | int | float | \DateInterval | Duration
+ * @psalm-type DateIntervalValue = string | int | float | \DateInterval | Duration | null
  */
 final class DateInterval
 {
@@ -115,6 +115,10 @@ public static function parse($interval, string $format = self::FORMAT_MILLISECON
                     $interval->getSeconds() * 1e6 + $interval->getNanos() / 1e3,
                     self::FORMAT_MICROSECONDS,
                 );
+
+            case $interval === null:
+                return CarbonInterval::create(0);
+
             default:
                 throw new \InvalidArgumentException(self::ERROR_INVALID_DATETIME);
         }

diff --git a/tests/Acceptance/Extra/TaskQueue/WorkflowATest.php b/tests/Acceptance/Extra/TaskQueue/WorkflowATest.php
@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Temporal\Tests\Acceptance\Extra\Workflow\WorkflowA;
+
+use PHPUnit\Framework\Attributes\Test;
+use Temporal\Client\WorkflowStubInterface;
+use Temporal\Tests\Acceptance\App\Attribute\Stub;
+use Temporal\Tests\Acceptance\App\TestCase;
+use Temporal\Workflow\WorkflowInterface;
+use Temporal\Workflow\WorkflowMethod;
+
+class WorkflowATest extends TestCase
+{
+    #[Test]
+    public function sendEmpty(
+        #[Stub(type: 'Workflow')]
+        WorkflowStubInterface $stub,
+    ): void {
+        $this->assertSame(42, $stub->getResult());
+    }
+}
+
+#[WorkflowInterface]
+class TestWorkflow
+{
+    #[WorkflowMethod(name: "Workflow")]
+    public function handle()
+    {
+        return 42;
+    }
+}
diff --git a/tests/Acceptance/Extra/TaskQueue/WorkflowBTest.php b/tests/Acceptance/Extra/TaskQueue/WorkflowBTest.php
@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Temporal\Tests\Acceptance\Extra\Workflow\WorkflowB;
+
+use PHPUnit\Framework\Attributes\CoversFunction;
+use PHPUnit\Framework\Attributes\Test;
+use Temporal\Client\WorkflowStubInterface;
+use Temporal\Tests\Acceptance\App\Attribute\Stub;
+use Temporal\Tests\Acceptance\App\TestCase;
+use Temporal\Workflow;
+use Temporal\Workflow\WorkflowInterface;
+use Temporal\Workflow\WorkflowMethod;
+
+class WorkflowBTest extends TestCase
+{
+    #[Test]
+    public function sendEmpty(
+        #[Stub(type: 'Workflow')]
+        WorkflowStubInterface $stub,
+    ): void {
+        $this->assertSame(24, $stub->getResult());
+    }
+}
+
+#[WorkflowInterface]
+class TestWorkflow
+{
+    #[WorkflowMethod(name: "Workflow")]
+    public function handle()
+    {
+        return 24;
+    }
+}
diff --git a/tests/Acceptance/Harness/ContinueAsNew/ContinueAsSameTest.php b/tests/Acceptance/Harness/ContinueAsNew/ContinueAsSameTest.php
@@ -27,7 +27,7 @@ public static function check(
             args: [INPUT_DATA],
             memo: [MEMO_KEY => MEMO_VALUE],
         )]
-        WorkflowStubInterface $stub
+        WorkflowStubInterface $stub,
     ): void {
         self::assertSame(INPUT_DATA, $stub->getResult());
         # Workflow ID does not change after continue as new

diff --git a/tests/Unit/Internal/Support/DateIntervalTestCase.php b/tests/Unit/Internal/Support/DateIntervalTestCase.php
@@ -12,12 +12,31 @@
 #[CoversClass(DateInterval::class)]
 final class DateIntervalTestCase extends TestCase
 {
+    public static function provideValuesToParse(): iterable
+    {
+        yield [1, DateInterval::FORMAT_MICROSECONDS, 1, '0/0/0/0'];
+        yield [1, DateInterval::FORMAT_MILLISECONDS, 1_000, '0/0/0/0'];
+        yield [0.25, DateInterval::FORMAT_SECONDS, 250_000, '0/0/0/0'];
+        yield [1, DateInterval::FORMAT_SECONDS, 1_000_000, '0/0/0/1'];
+        yield [1.25, DateInterval::FORMAT_SECONDS, 1_250_000, '0/0/0/1'];
+        yield [1.8, DateInterval::FORMAT_SECONDS, 1_800_000, '0/0/0/1'];
+        yield [1, DateInterval::FORMAT_MINUTES, 60_000_000, '0/0/1/0'];
+        yield [1.5, DateInterval::FORMAT_MINUTES, 90_000_000, '0/0/1/30'];
+        yield [1, DateInterval::FORMAT_HOURS, 3_600_000_000, '0/1/0/0'];
+        yield [1, DateInterval::FORMAT_DAYS, 86_400_000_000, '1/0/0/0'];
+        yield [1, DateInterval::FORMAT_WEEKS, 604_800_000_000, '7/0/0/0'];
+        yield [(0.1 + 0.7) * 10.0, DateInterval::FORMAT_SECONDS, 8_000_000, '0/0/0/8'];
+        yield [(0.1 + 0.7) * 10.0, DateInterval::FORMAT_DAYS, 691200000000, '8/0/0/0'];
+        yield [(0.1 + 0.7) * 10.0, DateInterval::FORMAT_WEEKS, 4838400000000, '56/0/0/0'];
+        yield [null, DateInterval::FORMAT_MILLISECONDS, 0, '0/0/0/0'];
+    }
+
     #[DataProvider('provideValuesToParse')]
     public function testParse(mixed $value, string $format, int $microseconds, string $formatted): void
     {
         $i = DateInterval::parse($value, $format);
 
-        self::assertSame($microseconds, (int)$i->totalMicroseconds);
+        self::assertSame($microseconds, (int) $i->totalMicroseconds);
         self::assertSame($formatted, $i->format('%d/%h/%i/%s'));
         if ($i->totalMicroseconds > 1_000_000) {
             self::assertGreaterThan(0, $i->totalSeconds);
@@ -28,7 +47,7 @@ public function testParseAndFormat(): void
     {
         $i = DateInterval::parse(6_200, DateInterval::FORMAT_MILLISECONDS);
 
-        $this->assertSame(6_200_000, (int)$i->totalMicroseconds);
+        $this->assertSame(6_200_000, (int) $i->totalMicroseconds);
         self::assertSame('0/0/0/6', $i->format('%y/%h/%i/%s'));
     }
 
@@ -40,25 +59,7 @@ public function testParseFromDuration(): void
 
         $i = DateInterval::parse($duration);
 
-        self::assertSame(5124, (int)$i->totalSeconds);
+        self::assertSame(5124, (int) $i->totalSeconds);
         self::assertSame(123_456, $i->microseconds);
     }
-
-    public static function provideValuesToParse(): iterable
-    {
-        yield [1, DateInterval::FORMAT_MICROSECONDS, 1, '0/0/0/0'];
-        yield [1, DateInterval::FORMAT_MILLISECONDS, 1_000, '0/0/0/0'];
-        yield [0.25, DateInterval::FORMAT_SECONDS, 250_000, '0/0/0/0'];
-        yield [1, DateInterval::FORMAT_SECONDS, 1_000_000, '0/0/0/1'];
-        yield [1.25, DateInterval::FORMAT_SECONDS, 1_250_000, '0/0/0/1'];
-        yield [1.8, DateInterval::FORMAT_SECONDS, 1_800_000, '0/0/0/1'];
-        yield [1, DateInterval::FORMAT_MINUTES, 60_000_000, '0/0/1/0'];
-        yield [1.5, DateInterval::FORMAT_MINUTES, 90_000_000, '0/0/1/30'];
-        yield [1, DateInterval::FORMAT_HOURS, 3_600_000_000, '0/1/0/0'];
-        yield [1, DateInterval::FORMAT_DAYS, 86_400_000_000, '1/0/0/0'];
-        yield [1, DateInterval::FORMAT_WEEKS, 604_800_000_000, '7/0/0/0'];
-        yield [(0.1 + 0.7) * 10.0, DateInterval::FORMAT_SECONDS, 8_000_000, '0/0/0/8'];
-        yield [(0.1 + 0.7) * 10.0, DateInterval::FORMAT_DAYS, 691200000000, '8/0/0/0'];
-        yield [(0.1 + 0.7) * 10.0, DateInterval::FORMAT_WEEKS, 4838400000000, '56/0/0/0'];
-    }
 }