Skip to content

Nested data objects with casts loses dataClass on $creationContext #1103

@piurafunk

Description

@piurafunk

✏️ Describe the bug
Working with JSON:API structure, I have a JSON string such as:

[
    'data' => [
        'attributes' => [
            'amountInCents' => 5_000_000,
        ],
        'id' => '29',
        'type' => 'cloudWalletTransactions',
    ],
    'jsonapi' => [
        'version' => '1.1',
    ],
    'meta' => [
        'cloudWalletBalanceInCents' => 10,
    ],
]

Several of these keys have custom casts, to ensure they cast to a derived instance of a parent Meta or Attributes object. When casting the top level meta entry, I intended to use the \Spatie\LaravelData\Support\Creation\CreationContext::$dataClass property to know which Document dervived class was being constructed, and thus which Meta derived class it should be cast to.

Somewhere along the line as it iterates over the previous keys, it pollutes the $dataClass property, changing the value to JsonApi in my case. This is because the jsonapi key prior to the meta key has its own Data class that is constructed, as far as I can tell. If I remove the jsonapi key from the array, the $dataClass property does not get polluted and it is available to check in my custom cast.

↪️ To Reproduce
My codebase is rather complex and so extracting the necessary pieces into a test seemed like a lot. I did make a small change and have the patch that seems to fix the problem:

diff --git a/src/Support/Creation/CreationContext_og.php b/src/Support/Creation/CreationContext.php
index 374fd1875..09809728e 100644
--- a/src/Support/Creation/CreationContext_og.php
+++ b/src/Support/Creation/CreationContext.php
@@ -23,6 +23,8 @@
  */
 class CreationContext
 {
+    protected array $dataClassHistory = [];
+
     /**
      * @param class-string<TData> $dataClass
      * @param array<string|int> $currentPath
@@ -77,6 +79,7 @@ public function next(
         string $dataClass,
         string|int $path,
     ): self {
+        $this->dataClassHistory[] = $this->dataClass;
         $this->dataClass = $dataClass;

         $this->currentPath[] = $path;
@@ -88,6 +91,7 @@ public function next(
     public function previous(): self
     {
         array_pop($this->currentPath);
+        $this->dataClass = array_pop($this->dataClassHistory);

         return $this;
     }

Essentially it just tracks the history of dataClass as next() and previous() are called, similar to how currentPath behaves.

I am willing to extract the necessary portions from my codebase and update this when I have time, but perhaps you guys will be faster at understanding a fix than I can be to provide a test.

This test shows the failure:

it ('does not pollute dataClass property', function() {
    class JsonApi extends Data implements Castable
    {
        public function __construct(public readonly string $version)
        {
        }

        public static function dataCastUsing(...$arguments): \Spatie\LaravelData\Casts\Cast
        {
            return new class () implements \Spatie\LaravelData\Casts\Cast {
                public function cast(
                    \Spatie\LaravelData\Support\DataProperty $property,
                    mixed $value,
                    array $properties,
                    \Spatie\LaravelData\Support\Creation\CreationContext $context
                ): JsonApi {
                    expect($context)->dataClass->toBe(DerivedDocument::class);
                    return JsonApi::from($value);
                }
            };
        }
    }

    abstract class Meta extends Data implements Castable
    {
        public static function dataCastUsing(...$arguments): \Spatie\LaravelData\Casts\Cast
        {
            return new class () implements \Spatie\LaravelData\Casts\Cast {
                public function cast(
                    \Spatie\LaravelData\Support\DataProperty $property,
                    mixed $value,
                    array $properties,
                    \Spatie\LaravelData\Support\Creation\CreationContext $context
                ): Meta {
                    expect($context->dataClass)->toBe(DerivedDocument::class);
                    return match ($context->dataClass) {
                        DerivedDocument::class => DerivedMeta::from($value),
                        default => throw new \InvalidArgumentException("Unknown Meta type: {$context->dataClass}"),
                    };
                }
            };
        }
    }

    class DerivedMeta extends Meta
    {
        public function __construct(public readonly string $copyright)
        {
        }
    }

    class Document extends Data
    {
        public function __construct(
            public readonly ?JsonApi $jsonapi = null,
            #[WithCastable(Meta::class)] public readonly ?Meta $meta = null,
        ) {
        }
    }

    class DerivedDocument extends Document
    {
        public function __construct(
            ?JsonApi $jsonapi = null,
            ?Meta $meta = null,
        ) {
            parent::__construct($jsonapi, $meta);
        }
    }

    $json = [
        'jsonapi' => [
            'version' => '1.0',
        ],
        'meta' => [
            'copyright' => 'Copyright 2024',
        ],
    ];

    $document = DerivedDocument::from($json);
    expect($document->meta)->toBeInstanceOf(DerivedMeta::class);
});

The line expect($context->dataClass)->toBe(DerivedDocument::class); can be removed from the abstract Meta class and it will instead throw the exception from the match() statement. In the $json array payload, removing the 'jsonapi' key or switching the order so that meta is first fixes it and the test passes.

✅ Expected behavior
$dataClass property on CreationContext does not get polluted.

🖥️ Versions

Laravel: v12.21.0
Laravel Data: 4.18.0
PHP: 8.2.29

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions