From 7551c3e0750aef5205a4a888cfa073e9466b8f43 Mon Sep 17 00:00:00 2001 From: Tofandel Date: Tue, 26 Mar 2024 18:20:16 +0100 Subject: [PATCH 1/5] Fix creation context issue Add failing test about nested collection lazy --- CHANGELOG.md | 2 +- src/DataPipes/CastPropertiesDataPipe.php | 6 ++- tests/CreationTest.php | 61 ++++++++++++++++++++++++ tests/PartialsTest.php | 26 ++++++++++ 4 files changed, 93 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6e8fcda..75ddac27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,7 +158,7 @@ Laravel-data 4.0.0 was released 5 hours ago, time for an update! - Make ValidationPath Stringable - Fix PHPStan -- Improve performance when optional peoperty exists (#612) +- Improve performance when optional property exists (#612) ## 3.10.0 - 2023-12-01 diff --git a/src/DataPipes/CastPropertiesDataPipe.php b/src/DataPipes/CastPropertiesDataPipe.php index e3b2c338..6099395b 100644 --- a/src/DataPipes/CastPropertiesDataPipe.php +++ b/src/DataPipes/CastPropertiesDataPipe.php @@ -87,9 +87,13 @@ protected function cast( ) { $context = $creationContext->next($property->type->dataClass, $property->name); - return $property->type->kind->isDataObject() + $data = $property->type->kind->isDataObject() ? $context->from($value) : $context->collect($value, $property->type->iterableClass); + + $creationContext->previous(); + + return $data; } if ( diff --git a/tests/CreationTest.php b/tests/CreationTest.php index 59c9ac37..82c77071 100644 --- a/tests/CreationTest.php +++ b/tests/CreationTest.php @@ -11,6 +11,10 @@ use Illuminate\Support\Facades\Route; use Illuminate\Validation\ValidationException; +use Spatie\LaravelData\DataPipeline; +use Spatie\LaravelData\DataPipes\DataPipe; +use Spatie\LaravelData\Support\Creation\CreationContext; +use Spatie\LaravelData\Support\DataClass; use function Pest\Laravel\postJson; use Spatie\LaravelData\Attributes\Computed; @@ -1067,3 +1071,60 @@ public function __invoke(SimpleData $data) ->toBeArray() ->toEqual(['a', 'collection']); })->skip(fn () => config('data.features.cast_and_transform_iterables') === false); + +it('keeps the creation context path up to date', function () { + global $testCreationContexts; + $testCreationContexts = []; + class TestDataPipe implements DataPipe + { + public function handle(mixed $payload, DataClass $class, array $properties, CreationContext $creationContext): array + { + global $testCreationContexts; + $testCreationContexts[] = clone $creationContext; + + return $properties; + } + } + class TestNestedDataPipe implements DataPipe + { + public function handle(mixed $payload, DataClass $class, array $properties, CreationContext $creationContext): array + { + global $testCreationContexts; + $testCreationContexts[] = clone $creationContext; + + return $properties; + } + } + + class SimpleDataWithTestPipe extends SimpleData + { + public static function pipeline(): DataPipeline + { + return parent::pipeline() + ->through(TestNestedDataPipe::class); + } + } + + $dataClass = new class () extends Data { + #[DataCollectionOf(SimpleDataWithTestPipe::class)] + public Collection $collection; + + public static function pipeline(): DataPipeline + { + return parent::pipeline() + ->through(TestDataPipe::class); + } + }; + + $dataClass::from([ + 'collection' => [['string' => 'no'], 'models', ['string' => 'here']], + ]); + + expect($testCreationContexts)->toHaveCount(3); + expect($testCreationContexts[0])->toBeInstanceOf(CreationContext::class); + + expect($testCreationContexts[0]->currentPath)->toBe([0 => 'collection', 1 => 0]); + expect($testCreationContexts[1]->currentPath)->toBe([0 => 'collection', 1 => 2]); + expect($testCreationContexts[2]->currentPath)->toHaveCount(0); + +}); diff --git a/tests/PartialsTest.php b/tests/PartialsTest.php index f6a8ef13..8619d084 100644 --- a/tests/PartialsTest.php +++ b/tests/PartialsTest.php @@ -225,6 +225,32 @@ ]); }); +it('can conditionally include nested collection', function () { + $dataClass = new class () extends Data { + #[DataCollectionOf(MultiLazyData::class)] + public Collection $nested; + }; + + $data = $dataClass::collect([ + [ + 'nested' => [DummyDto::rick()], + ], [ + 'nested' => [DummyDto::bon()], + ] + ], DataCollection::class); + + expect($data->toArray())->toMatchArray([ + ['nested' => [[]]], + ['nested' => [[]]] + ]); + + expect($data->include('nested.{artist,year}')->toArray()) + ->toMatchArray([ + ['nested' => [['artist' => DummyDto::rick()->artist, 'year' => DummyDto::rick()->year]]], + ['nested' => [['artist' => DummyDto::bon()->artist, 'year' => DummyDto::bon()->year]]] + ]); +}); + it('can conditionally include using class defaults', function () { PartialClassConditionalData::setDefinitions(includeDefinitions: [ 'string' => fn (PartialClassConditionalData $data) => $data->enabled, From 0ee52b1c85ad92d303f6732a0076baf6cb1b12e3 Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Fri, 3 May 2024 12:47:01 +0200 Subject: [PATCH 2/5] Remove unnecessary --- tests/CreationTest.php | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/tests/CreationTest.php b/tests/CreationTest.php index 5ed72909..2a6792a3 100644 --- a/tests/CreationTest.php +++ b/tests/CreationTest.php @@ -1085,23 +1085,13 @@ public function handle(mixed $payload, DataClass $class, array $properties, Crea return $properties; } } - class TestNestedDataPipe implements DataPipe - { - public function handle(mixed $payload, DataClass $class, array $properties, CreationContext $creationContext): array - { - global $testCreationContexts; - $testCreationContexts[] = clone $creationContext; - - return $properties; - } - } class SimpleDataWithTestPipe extends SimpleData { public static function pipeline(): DataPipeline { return parent::pipeline() - ->through(TestNestedDataPipe::class); + ->through(TestDataPipe::class); } } From 4ff8e75c09a818fc106f11d268cdc8319a7a880e Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Fri, 3 May 2024 13:28:41 +0200 Subject: [PATCH 3/5] Fix missing return in merge commit --- src/DataPipes/CastPropertiesDataPipe.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/DataPipes/CastPropertiesDataPipe.php b/src/DataPipes/CastPropertiesDataPipe.php index b0b83fc2..78a1aae1 100644 --- a/src/DataPipes/CastPropertiesDataPipe.php +++ b/src/DataPipes/CastPropertiesDataPipe.php @@ -94,6 +94,7 @@ protected function cast( : $context->collect($value, $property->type->iterableClass); $creationContext->previous(); + return $data; } catch (CannotCreateData) { return $value; } From 5da2d7e63c60b68e4d4cbe459bd6d0cb3b05a18b Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Fri, 3 May 2024 13:29:05 +0200 Subject: [PATCH 4/5] Update src/DataPipes/CastPropertiesDataPipe.php --- src/DataPipes/CastPropertiesDataPipe.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/DataPipes/CastPropertiesDataPipe.php b/src/DataPipes/CastPropertiesDataPipe.php index 78a1aae1..1e1a0723 100644 --- a/src/DataPipes/CastPropertiesDataPipe.php +++ b/src/DataPipes/CastPropertiesDataPipe.php @@ -94,7 +94,8 @@ protected function cast( : $context->collect($value, $property->type->iterableClass); $creationContext->previous(); - return $data; + + return $data; } catch (CannotCreateData) { return $value; } From d32c8e273eb9b43c75ebd0c43ed91628664da5e4 Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Fri, 3 May 2024 13:30:14 +0200 Subject: [PATCH 5/5] Fix indent --- src/DataPipes/CastPropertiesDataPipe.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/DataPipes/CastPropertiesDataPipe.php b/src/DataPipes/CastPropertiesDataPipe.php index 1e1a0723..e8bfe74d 100644 --- a/src/DataPipes/CastPropertiesDataPipe.php +++ b/src/DataPipes/CastPropertiesDataPipe.php @@ -87,15 +87,15 @@ protected function cast( || $property->type->kind->isDataCollectable() ) { try { - $context = $creationContext->next($property->type->dataClass, $property->name); + $context = $creationContext->next($property->type->dataClass, $property->name); - $data = $property->type->kind->isDataObject() - ? $context->from($value) - : $context->collect($value, $property->type->iterableClass); + $data = $property->type->kind->isDataObject() + ? $context->from($value) + : $context->collect($value, $property->type->iterableClass); - $creationContext->previous(); + $creationContext->previous(); - return $data; + return $data; } catch (CannotCreateData) { return $value; }