serializer: codec-independent helpers and Throwable normalization

durable-workflow-ops · durable-workflow-ops · commit 07beb4144d4d · 2026-04-17T08:54:11.000Z
Before this change `Serializer::serializable()`, `Serializer::serializeModels()`, and `Serializer::unserializeModels()` routed through `Serializer::__callStatic` to the configured codec. Since the v2 default moved to `json` (and then `avro`), neither of which implement those helper methods, the dispatch fell through to the `method_exists(...)` guard and silently returned `null`. That caused v2 failure normalization (`FailureFactory::normalizeProperties` / `restoreThrowable`) and v1 trace filtering (`WorkflowStub::fail`, `Activity::fail`) to drop property values and trace frames whenever the default codec did not inherit from `AbstractSerializer`. Changes: - Add `Serializer::serializable()`, `Serializer::serializeModels()`, and `Serializer::unserializeModels()` as first-class static methods backed by a local `ModelIdentifierHelper` that reuses Laravel's `SerializesAndRestoresModelIdentifiers` trait. These short-circuit before `__callStatic`, so they behave the same under every codec. - Pre-normalize Throwable/model arguments in `Serializer::__callStatic('serialize', ...)` and `Serializer::serializeWithCodec()` whenever the chosen codec is not an `AbstractSerializer` subclass. Legacy codecs still double-normalize via `AbstractSerializer::serialize()` (array in, array out), which is safe. - Replace the `$class::getInstance()` dispatch in `__callStatic` with `method_exists($class, $name) && $class::{$name}(...)`. `Avro` (registered by 1327f11) does not provide `getInstance()`, so the instance-based path was broken for the new default codec. - Update `WorkflowsConfigTest` to expect the current default `avro`. - Add `CodecIndependentHelpersTest` covering serializable / serializeModels / unserializeModels / direct Throwable round-trip / legacy Y+Base64 encode/decode across every registered codec. Closes #335.
diff --git a/src/Serializers/Serializer.php b/src/Serializers/Serializer.php
@@ -4,40 +4,103 @@
 
 namespace Workflow\Serializers;
 
+use Illuminate\Queue\SerializesAndRestoresModelIdentifiers;
+use Throwable;
+
 final class Serializer
 {
     /**
-     * Legacy magic dispatch — preserves the pre-codec-registry behavior.
+     * Shared codec-independent normalization helpers live on this object so
+     * that static calls like {@see Serializer::serializable()} can reach them
+     * without coupling callers to the configured codec.
+     */
+    private static ?ModelIdentifierHelper $helper = null;
+
+    /**
+     * Legacy magic dispatch — preserves the pre-codec-registry behavior for
+     * the codec-specific surface: {@see serialize()} / {@see unserialize()}.
+     *
+     * - serialize(): uses config('workflows.serializer') (default "json").
+     * - unserialize(): sniffs the blob ("base64:" prefix → Base64, JSON-like →
+     *   Json, else Y).
      *
-     * - serialize(): uses config('workflows.serializer') (default Y).
-     * - unserialize(): sniffs the blob ("base64:" prefix → Base64, else Y).
+     * Codec-independent helpers ({@see serializable()}, {@see serializeModels()},
+     * {@see unserializeModels()}) are declared as first-class static methods
+     * on this class and short-circuit before __callStatic so they produce the
+     * same result regardless of the configured codec. That is important for
+     * the JSON default: {@see Json} does not implement those helpers, and
+     * silently returning null from them used to drop exception trace frames
+     * and failure-property values during v2 failure normalization.
      *
      * New code should prefer {@see self::serializeWithCodec()} /
      * {@see self::unserializeWithCodec()} which make the codec choice explicit.
      */
     public static function __callStatic(string $name, array $arguments)
     {
         if ($name === 'unserialize') {
-            $instance = self::legacyUnserializeInstance((string) ($arguments[0] ?? ''));
+            $class = self::legacyUnserializeClass((string) ($arguments[0] ?? ''));
         } else {
-            $instance = self::defaultInstance();
+            $class = self::defaultCodecClass();
         }
 
-        if (method_exists($instance, $name)) {
-            return $instance->{$name}(...$arguments);
+        if ($name === 'serialize' && ! is_subclass_of($class, AbstractSerializer::class)) {
+            $arguments[0] = self::normalizeForCodec($arguments[0] ?? null);
+        }
+
+        if (method_exists($class, $name)) {
+            return $class::{$name}(...$arguments);
         }
     }
 
-    private static function defaultInstance(): SerializerInterface
+    /**
+     * Codec-independent replacement for the legacy AbstractSerializer helper:
+     * is this value safe to pass to PHP's native serialize()?
+     *
+     * Used by exception-trace filtering and v2 failure property capture. Must
+     * be safe to call regardless of the configured codec.
+     */
+    public static function serializable(mixed $data): bool
     {
-        $configured = function_exists('config') ? config('workflows.serializer') : null;
+        try {
+            serialize($data);
+            return true;
+        } catch (Throwable) {
+            return false;
+        }
+    }
 
-        if (is_string($configured) && $configured !== '') {
-            $class = CodecRegistry::resolve($configured);
-            return $class::getInstance();
+    /**
+     * Recursively replace Eloquent models inside $data with their serialized
+     * identifier representation, and convert Throwable instances into plain
+     * arrays. Always applied by v1 and v2 failure normalization paths.
+     *
+     * Codec-independent: returns the same shape regardless of whether the
+     * configured codec is "json", Y, or Base64.
+     */
+    public static function serializeModels(mixed $data): mixed
+    {
+        if ($data instanceof Throwable) {
+            return self::throwableToArray($data);
         }
 
-        return Json::getInstance();
+        if (is_array($data)) {
+            return self::helper()->serializeValue($data);
+        }
+
+        return $data;
+    }
+
+    /**
+     * Inverse of {@see serializeModels()} for nested arrays. Scalars and
+     * non-array values are returned unchanged.
+     */
+    public static function unserializeModels(mixed $data): mixed
+    {
+        if (is_array($data)) {
+            return self::helper()->unserializeValue($data);
+        }
+
+        return $data;
     }
 
     /**
@@ -48,6 +111,11 @@ private static function defaultInstance(): SerializerInterface
     public static function serializeWithCodec(?string $codec, $data): string
     {
         $class = CodecRegistry::resolve($codec);
+
+        if (! is_subclass_of($class, AbstractSerializer::class)) {
+            $data = self::normalizeForCodec($data);
+        }
+
         return $class::serialize($data);
     }
 
@@ -60,19 +128,74 @@ public static function unserializeWithCodec(?string $codec, string $data)
         return $class::unserialize($data);
     }
 
-    private static function legacyUnserializeInstance(string $blob): SerializerInterface
+    /**
+     * @return class-string<SerializerInterface>
+     */
+    private static function defaultCodecClass(): string
+    {
+        $configured = function_exists('config') ? config('workflows.serializer') : null;
+
+        if (is_string($configured) && $configured !== '') {
+            return CodecRegistry::resolve($configured);
+        }
+
+        return CodecRegistry::resolve(null);
+    }
+
+    /**
+     * Pre-normalize $data before handing it to a codec that does not itself
+     * apply model/Throwable normalization (for example {@see Json}). Legacy
+     * codecs that extend {@see AbstractSerializer} already call serializeModels
+     * internally and must not be double-normalized here.
+     */
+    private static function normalizeForCodec(mixed $data): mixed
+    {
+        return self::serializeModels($data);
+    }
+
+    /**
+     * @return array{class: class-string<Throwable>, message: string, code: int|string, line: int, file: string, trace: list<array<string, mixed>>}
+     */
+    private static function throwableToArray(Throwable $throwable): array
+    {
+        return [
+            'class' => get_class($throwable),
+            'message' => $throwable->getMessage(),
+            'code' => $throwable->getCode(),
+            'line' => $throwable->getLine(),
+            'file' => $throwable->getFile(),
+            'trace' => collect($throwable->getTrace())
+                ->filter(static fn ($trace) => self::serializable($trace))
+                ->values()
+                ->toArray(),
+        ];
+    }
+
+    private static function helper(): ModelIdentifierHelper
+    {
+        if (self::$helper === null) {
+            self::$helper = new ModelIdentifierHelper();
+        }
+
+        return self::$helper;
+    }
+
+    /**
+     * @return class-string<SerializerInterface>
+     */
+    private static function legacyUnserializeClass(string $blob): string
     {
         if (str_starts_with($blob, 'base64:')) {
-            return Base64::getInstance();
+            return Base64::class;
         }
 
         // JSON blobs always start with "{", "[", a digit, quote, minus, "t"/"f"/"n".
         // PHP-serialized-closure blobs start with "O:".
         if ($blob !== '' && $blob[0] !== 'O' && self::looksLikeJson($blob)) {
-            return Json::getInstance();
+            return Json::class;
         }
 
-        return Y::getInstance();
+        return Y::class;
     }
 
     private static function looksLikeJson(string $blob): bool
@@ -87,3 +210,40 @@ private static function looksLikeJson(string $blob): bool
         return in_array($blob, ['true', 'false', 'null'], true);
     }
 }
+
+/**
+ * @internal
+ */
+final class ModelIdentifierHelper
+{
+    use SerializesAndRestoresModelIdentifiers {
+        getSerializedPropertyValue as public;
+        getRestoredPropertyValue as public;
+    }
+
+    public function serializeValue(mixed $value): mixed
+    {
+        if (is_array($value)) {
+            foreach ($value as $key => $nested) {
+                $value[$key] = $this->serializeValue($nested);
+            }
+
+            return $value;
+        }
+
+        return $this->getSerializedPropertyValue($value);
+    }
+
+    public function unserializeValue(mixed $value): mixed
+    {
+        if (is_array($value)) {
+            foreach ($value as $key => $nested) {
+                $value[$key] = $this->unserializeValue($nested);
+            }
+
+            return $value;
+        }
+
+        return $this->getRestoredPropertyValue($value);
+    }
+}
diff --git a/tests/Unit/Config/WorkflowsConfigTest.php b/tests/Unit/Config/WorkflowsConfigTest.php
@@ -22,7 +22,7 @@ public function testConfigIsLoaded(): void
             'stored_workflow_signal_model' => \Workflow\Models\StoredWorkflowSignal::class,
             'stored_workflow_timer_model' => \Workflow\Models\StoredWorkflowTimer::class,
             'workflow_relationships_table' => 'workflow_relationships',
-            'serializer' => 'json',
+            'serializer' => 'avro',
             'prune_age' => '1 month',
             'webhooks_route' => env('WORKFLOW_WEBHOOKS_ROUTE', 'webhooks'),
         ];
diff --git a/tests/Unit/Serializers/CodecIndependentHelpersTest.php b/tests/Unit/Serializers/CodecIndependentHelpersTest.php
@@ -0,0 +1,133 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Unit\Serializers;
+
+use Exception;
+use Tests\TestCase;
+use Workflow\Serializers\Base64;
+use Workflow\Serializers\Json;
+use Workflow\Serializers\Serializer;
+use Workflow\Serializers\Y;
+
+final class CodecIndependentHelpersTest extends TestCase
+{
+    /**
+     * @dataProvider codecProvider
+     */
+    public function testSerializableReturnsTrueForScalarsRegardlessOfCodec(string $codec): void
+    {
+        config(['workflows.serializer' => $codec]);
+
+        $this->assertTrue(Serializer::serializable('foo'));
+        $this->assertTrue(Serializer::serializable(42));
+        $this->assertTrue(Serializer::serializable(['a' => 1]));
+        $this->assertTrue(Serializer::serializable(null));
+    }
+
+    /**
+     * @dataProvider codecProvider
+     */
+    public function testSerializableReturnsFalseForClosureRegardlessOfCodec(string $codec): void
+    {
+        config(['workflows.serializer' => $codec]);
+
+        $this->assertFalse(Serializer::serializable(static fn (): string => 'closure'));
+    }
+
+    /**
+     * @dataProvider codecProvider
+     */
+    public function testSerializeModelsPassesThroughPlainArraysRegardlessOfCodec(string $codec): void
+    {
+        config(['workflows.serializer' => $codec]);
+
+        $input = ['a' => 1, 'b' => ['nested' => true]];
+
+        $this->assertSame($input, Serializer::serializeModels($input));
+    }
+
+    /**
+     * @dataProvider codecProvider
+     */
+    public function testSerializeModelsConvertsThrowableToArrayRegardlessOfCodec(string $codec): void
+    {
+        config(['workflows.serializer' => $codec]);
+
+        $throwable = new Exception('boom', 7);
+        $data = Serializer::serializeModels($throwable);
+
+        $this->assertIsArray($data);
+        $this->assertSame(Exception::class, $data['class']);
+        $this->assertSame('boom', $data['message']);
+        $this->assertSame(7, $data['code']);
+        $this->assertArrayHasKey('trace', $data);
+        $this->assertIsArray($data['trace']);
+    }
+
+    /**
+     * @dataProvider codecProvider
+     */
+    public function testUnserializeModelsIsIdentityForPlainArraysRegardlessOfCodec(string $codec): void
+    {
+        config(['workflows.serializer' => $codec]);
+
+        $input = ['a' => 1, 'b' => ['c' => 'x']];
+
+        $this->assertSame($input, Serializer::unserializeModels($input));
+    }
+
+    /**
+     * @dataProvider languageNeutralCodecProvider
+     */
+    public function testSerializeThrowableUnderLanguageNeutralCodecPreservesDiagnosticData(string $codec): void
+    {
+        if ($codec === 'avro' && ! class_exists(\Apache\Avro\Schema\AvroSchema::class)) {
+            $this->markTestSkipped('apache/avro package is not installed in this environment.');
+        }
+
+        config(['workflows.serializer' => $codec]);
+
+        $throwable = new Exception('boom', 9);
+        $serialized = Serializer::serialize($throwable);
+        $decoded = Serializer::unserialize($serialized);
+
+        $this->assertIsArray($decoded);
+        $this->assertSame(Exception::class, $decoded['class']);
+        $this->assertSame('boom', $decoded['message']);
+        $this->assertSame(9, $decoded['code']);
+        $this->assertArrayHasKey('trace', $decoded);
+        $this->assertIsArray($decoded['trace']);
+    }
+
+    public static function languageNeutralCodecProvider(): array
+    {
+        return [
+            'json' => ['json'],
+            'avro' => ['avro'],
+        ];
+    }
+
+    public function testLegacyCodecsRoundTripBytesThroughEncodeDecode(): void
+    {
+        foreach ([Y::class, Base64::class] as $codec) {
+            config(['workflows.serializer' => $codec]);
+
+            $bytes = "\x00\x01binary" . random_bytes(32);
+            $roundTripped = Serializer::decode(Serializer::encode($bytes));
+
+            $this->assertSame($bytes, $roundTripped, "Codec {$codec} must round-trip raw bytes");
+        }
+    }
+
+    public static function codecProvider(): array
+    {
+        return [
+            'json' => ['json'],
+            'avro' => ['avro'],
+            'Y' => [Y::class],
+            'Base64' => [Base64::class],
+        ];
+    }
+}
