From 884e935bd041716d563c0ef90588ba686882f64a Mon Sep 17 00:00:00 2001 From: Ruben Van Assche Date: Thu, 16 Mar 2023 17:55:28 +0100 Subject: [PATCH 1/7] Add benchmarking --- .gitignore | 2 ++ benchmarks/DataBench.php | 74 ++++++++++++++++++++++++++++++++++++++++ phpbench.json | 5 +++ phpbench.json.dist | 9 ----- 4 files changed, 81 insertions(+), 9 deletions(-) create mode 100644 benchmarks/DataBench.php create mode 100644 phpbench.json delete mode 100644 phpbench.json.dist diff --git a/.gitignore b/.gitignore index 1cf475ff..fe98f112 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,5 @@ testbench.yaml vendor node_modules .php-cs-fixer.cache +.phpbench +.DS_Store diff --git a/benchmarks/DataBench.php b/benchmarks/DataBench.php new file mode 100644 index 00000000..97ea3f28 --- /dev/null +++ b/benchmarks/DataBench.php @@ -0,0 +1,74 @@ +createApplication(); + } + + protected function getPackageProviders($app) + { + return [ + LaravelDataServiceProvider::class, + ]; + } + + #[Revs(500), Iterations(2)] + public function benchData() + { + MultiNestedData::from([ + 'nested' => ['simple' => 'Hello'], + 'nestedCollection' => [ + ['simple' => 'I'], + ['simple' => 'am'], + ['simple' => 'groot'], + ], + ]); + } + + #[Revs(500), Iterations(2)] + public function benchDataCollection() + { + $collection = Collection::times( + 15, + fn() => [ + 'withoutType' => 42, + 'int' => 42, + 'bool' => true, + 'float' => 3.14, + 'string' => 'Hello world', + 'array' => [1, 1, 2, 3, 5, 8], + 'nullable' => null, + 'mixed' => 42, + 'explicitCast' => '16-06-1994', + 'defaultCast' => '1994-05-16T12:00:00+01:00', + 'nestedData' => [ + 'string' => 'hello', + ], + 'nestedCollection' => [ + ['string' => 'never'], + ['string' => 'gonna'], + ['string' => 'give'], + ['string' => 'you'], + ['string' => 'up'], + ], + ] + )->all(); + + ComplicatedData::collection($collection); + } +} diff --git a/phpbench.json b/phpbench.json new file mode 100644 index 00000000..f171580e --- /dev/null +++ b/phpbench.json @@ -0,0 +1,5 @@ +{ + "$schema":"./vendor/phpbench/phpbench/phpbench.schema.json", + "runner.bootstrap": "vendor/autoload.php", + "runner.path" : "benchmarks" +} diff --git a/phpbench.json.dist b/phpbench.json.dist deleted file mode 100644 index b4d4f77d..00000000 --- a/phpbench.json.dist +++ /dev/null @@ -1,9 +0,0 @@ -{ - "php_disable_ini": true, - "bootstrap": "vendor/autoload.php", - "path": "benchmark", - "php_config": { - "extension": [ "json.so" ] - }, - "time_unit": "milliseconds" -} From f08eaa36c8a8fd4a8c05211009c2a613b26dcc22 Mon Sep 17 00:00:00 2001 From: Ruben Van Assche Date: Thu, 16 Mar 2023 17:56:02 +0100 Subject: [PATCH 2/7] Add pipeline changes --- src/DataPipeline.php | 15 +++++--- src/Resolvers/DataFromSomethingResolver.php | 39 +++++--------------- src/Resolvers/EmptyDataResolver.php | 8 ++-- src/Support/DataConfig.php | 23 ++++++++++++ src/Transformers/DataTransformer.php | 28 -------------- tests/TestSupport/DataValidationAsserter.php | 1 + 6 files changed, 49 insertions(+), 65 deletions(-) diff --git a/src/DataPipeline.php b/src/DataPipeline.php index beea1d4a..d13cc224 100644 --- a/src/DataPipeline.php +++ b/src/DataPipeline.php @@ -62,23 +62,28 @@ public function firstThrough(string|DataPipe $pipe): static return $this; } - public function execute(): Collection + public function prepare(): self { /** @var \Spatie\LaravelData\Normalizers\Normalizer[] $normalizers */ - $normalizers = array_map( + $this->normalizers = array_map( fn (string|Normalizer $normalizer) => is_string($normalizer) ? app($normalizer) : $normalizer, $this->normalizers ); /** @var \Spatie\LaravelData\DataPipes\DataPipe[] $pipes */ - $pipes = array_map( + $this->pipes = array_map( fn (string|DataPipe $pipe) => is_string($pipe) ? app($pipe) : $pipe, $this->pipes ); + return $this; + } + + public function execute(): Collection + { $properties = null; - foreach ($normalizers as $normalizer) { + foreach ($this->normalizers as $normalizer) { $properties = $normalizer->normalize($this->value); if ($properties !== null) { @@ -96,7 +101,7 @@ public function execute(): Collection $properties = ($class->name)::prepareForPipeline($properties); - foreach ($pipes as $pipe) { + foreach ($this->pipes as $pipe) { $piped = $pipe->handle($this->value, $class, $properties); $properties = $piped; diff --git a/src/Resolvers/DataFromSomethingResolver.php b/src/Resolvers/DataFromSomethingResolver.php index f8f03f7c..e6e38146 100644 --- a/src/Resolvers/DataFromSomethingResolver.php +++ b/src/Resolvers/DataFromSomethingResolver.php @@ -39,35 +39,15 @@ public function execute(string $class, mixed ...$payloads): BaseData return $data; } -// $properties = new Collection(); -// -// foreach ($payloads as $payload) { -// /** @var BaseData $class */ -// $pipeline = $class::pipeline(); -// -// foreach ($class::normalizers() as $normalizer) { -// $pipeline->normalizer($normalizer); -// } -// -// foreach ($pipeline->using($payload)->execute() as $key => $value) { -// $properties[$key] = $value; -// } -// } - - $properties = array_reduce( - $payloads, - function (Collection $carry, mixed $payload) use ($class) { - /** @var BaseData $class */ - $pipeline = $class::pipeline(); - - foreach ($class::normalizers() as $normalizer) { - $pipeline->normalizer($normalizer); - } - - return $carry->merge($pipeline->using($payload)->execute()); - }, - collect(), - ); + $properties = new Collection(); + + $pipeline = $this->dataConfig->getDataPipeLine($class); + + foreach ($payloads as $payload) { + foreach ($pipeline->using($payload)->execute() as $key => $value) { + $properties[$key] = $value; + } + } return $this->dataFromArrayResolver->execute($class, $properties); } @@ -107,6 +87,7 @@ protected function createFromCustomCreationMethod(string $class, array $payloads ->normalizer(ArrayableNormalizer::class) ->into($class) ->using($payload) + ->prepare() ->execute(); } } diff --git a/src/Resolvers/EmptyDataResolver.php b/src/Resolvers/EmptyDataResolver.php index b67ed410..9bab4539 100644 --- a/src/Resolvers/EmptyDataResolver.php +++ b/src/Resolvers/EmptyDataResolver.php @@ -17,7 +17,9 @@ public function execute(string $class, array $extra = []): array { $dataClass = $this->dataConfig->getDataClass($class); - return $dataClass->properties->reduce(function (array $payload, DataProperty $property) use ($extra) { + $payload = []; + + foreach ($dataClass->properties as $property){ $name = $property->outputMappedName ?? $property->name; if ($property->hasDefaultValue) { @@ -25,9 +27,9 @@ public function execute(string $class, array $extra = []): array } else { $payload[$name] = $extra[$property->name] ?? $this->getValueForProperty($property); } + } - return $payload; - }, []); + return $payload; } protected function getValueForProperty(DataProperty $property): mixed diff --git a/src/Support/DataConfig.php b/src/Support/DataConfig.php index e35e8219..587bfe39 100644 --- a/src/Support/DataConfig.php +++ b/src/Support/DataConfig.php @@ -4,6 +4,8 @@ use ReflectionClass; use Spatie\LaravelData\Casts\Cast; +use Spatie\LaravelData\Contracts\BaseData; +use Spatie\LaravelData\DataPipeline; use Spatie\LaravelData\Transformers\Transformer; class DataConfig @@ -17,6 +19,9 @@ class DataConfig /** @var array */ protected array $casts = []; + /** @var array */ + protected array $dataPipelines = []; + /** @var \Spatie\LaravelData\RuleInferrers\RuleInferrer[] */ protected array $ruleInferrers; @@ -45,6 +50,24 @@ public function getDataClass(string $class): DataClass return $this->dataClasses[$class] = DataClass::create(new ReflectionClass($class)); } + public function getDataPipeLine(string $class): DataPipeline + { + if(array_key_exists($class, $this->dataPipelines)){ + return $this->dataPipelines[$class]; + } + + /** @var BaseData $class */ + $pipeline = $class::pipeline(); + + foreach ($class::normalizers() as $normalizer) { + $pipeline->normalizer($normalizer); + } + + $pipeline->prepare(); + + return $this->dataPipelines[$class] = $pipeline; + } + public function findGlobalCastForProperty(DataProperty $property): ?Cast { foreach ($property->type->acceptedTypes as $acceptedType => $baseTypes) { diff --git a/src/Transformers/DataTransformer.php b/src/Transformers/DataTransformer.php index a55c81c8..63f1cf35 100644 --- a/src/Transformers/DataTransformer.php +++ b/src/Transformers/DataTransformer.php @@ -94,34 +94,6 @@ protected function resolvePayload(TransformableData $data): array } return $payload; - -// return $dataClass -// ->properties -// ->reduce(function (array $payload, DataProperty $property) use ($data, $trees) { -// $name = $property->name; -// -// if (! $this->shouldIncludeProperty($name, $data->{$name}, $trees)) { -// return $payload; -// } -// -// $value = $this->resolvePropertyValue( -// $property, -// $data->{$name}, -// $trees->getNested($name), -// ); -// -// if ($value instanceof Optional) { -// return $payload; -// } -// -// if ($this->mapPropertyNames && $property->outputMappedName) { -// $name = $property->outputMappedName; -// } -// -// $payload[$name] = $value; -// -// return $payload; -// }, []); } protected function shouldIncludeProperty( diff --git a/tests/TestSupport/DataValidationAsserter.php b/tests/TestSupport/DataValidationAsserter.php index 93017d04..c1669940 100644 --- a/tests/TestSupport/DataValidationAsserter.php +++ b/tests/TestSupport/DataValidationAsserter.php @@ -163,6 +163,7 @@ private function pipePayload(array $payload): array ->into($this->dataClass) ->through(MapPropertiesDataPipe::class) ->through(ValidatePropertiesDataPipe::class) + ->prepare() ->execute(); return $properties->all(); From 9fd206a0cf1d2aa76f699209cf8f5d8211b6000c Mon Sep 17 00:00:00 2001 From: Ruben Van Assche Date: Fri, 17 Mar 2023 10:14:12 +0100 Subject: [PATCH 3/7] Performance improvements --- composer.json | 38 ++++++------ src/DataPipeline.php | 62 +++++++------------- src/Resolvers/DataFromSomethingResolver.php | 13 ++-- src/Support/DataConfig.php | 29 +++++---- src/Support/ResolvedDataPipeline.php | 49 ++++++++++++++++ tests/TestSupport/DataValidationAsserter.php | 5 +- tests/ValidationTest.php | 3 + 7 files changed, 114 insertions(+), 85 deletions(-) create mode 100644 src/Support/ResolvedDataPipeline.php diff --git a/composer.json b/composer.json index 25811b02..41854ccc 100644 --- a/composer.json +++ b/composer.json @@ -22,23 +22,23 @@ "spatie/laravel-package-tools" : "^1.9.0" }, "require-dev" : { - "fakerphp/faker": "^1.14", - "friendsofphp/php-cs-fixer": "^3.0", - "inertiajs/inertia-laravel": "^0.6.3", - "nesbot/carbon": "^2.63", - "nette/php-generator": "^3.5", - "nunomaduro/larastan": "^2.0", - "orchestra/testbench": "^7.6|^8.0", - "pestphp/pest": "^1.22", - "pestphp/pest-plugin-laravel": "^1.3", - "phpbench/phpbench": "^1.2", - "phpstan/extension-installer": "^1.1", - "phpunit/phpunit": "^9.3", - "spatie/invade": "^1.0", - "spatie/laravel-typescript-transformer": "^2.1.6", - "spatie/pest-plugin-snapshots": "^1.1", - "spatie/phpunit-snapshot-assertions": "^4.2", - "spatie/test-time": "^1.2" + "fakerphp/faker" : "^1.14", + "friendsofphp/php-cs-fixer" : "^3.0", + "inertiajs/inertia-laravel" : "^0.6.3", + "nesbot/carbon" : "^2.63", + "nette/php-generator" : "^3.5", + "nunomaduro/larastan" : "^2.0", + "orchestra/testbench" : "^7.6|^8.0", + "pestphp/pest" : "^1.22", + "pestphp/pest-plugin-laravel" : "^1.3", + "phpbench/phpbench" : "^1.2", + "phpstan/extension-installer" : "^1.1", + "phpunit/phpunit" : "^9.3", + "spatie/invade" : "^1.0", + "spatie/laravel-typescript-transformer" : "^2.1.6", + "spatie/pest-plugin-snapshots" : "^1.1", + "spatie/phpunit-snapshot-assertions" : "^4.2", + "spatie/test-time" : "^1.2" }, "autoload" : { "psr-4" : { @@ -55,7 +55,9 @@ "analyse" : "vendor/bin/phpstan analyse", "test" : "./vendor/bin/pest --no-coverage", "test-coverage" : "vendor/bin/pest --coverage-html coverage", - "format" : "vendor/bin/php-cs-fixer fix --allow-risky=yes" + "format" : "vendor/bin/php-cs-fixer fix --allow-risky=yes", + "benchmark" : "vendor/bin/phpbench run --report=default", + "benchmark-profiled" : "vendor/bin/phpbench xdebug:profile" }, "config" : { "sort-packages" : true, diff --git a/src/DataPipeline.php b/src/DataPipeline.php index d13cc224..e8e3e416 100644 --- a/src/DataPipeline.php +++ b/src/DataPipeline.php @@ -7,6 +7,7 @@ use Spatie\LaravelData\Exceptions\CannotCreateData; use Spatie\LaravelData\Normalizers\Normalizer; use Spatie\LaravelData\Support\DataConfig; +use Spatie\LaravelData\Support\ResolvedDataPipeline; class DataPipeline { @@ -27,13 +28,6 @@ public static function create(): static return app(static::class); } - public function using(mixed $value): static - { - $this->value = $value; - - return $this; - } - public function into(string $classString): static { $this->classString = $classString; @@ -62,51 +56,37 @@ public function firstThrough(string|DataPipe $pipe): static return $this; } - public function prepare(): self + public function resolve(): ResolvedDataPipeline { + $normalizers = array_merge( + $this->normalizers, + $this->classString::normalizers() + ); + /** @var \Spatie\LaravelData\Normalizers\Normalizer[] $normalizers */ - $this->normalizers = array_map( - fn (string|Normalizer $normalizer) => is_string($normalizer) ? app($normalizer) : $normalizer, - $this->normalizers + $normalizers = array_map( + fn(string|Normalizer $normalizer) => is_string($normalizer) ? app($normalizer) : $normalizer, + $normalizers ); /** @var \Spatie\LaravelData\DataPipes\DataPipe[] $pipes */ - $this->pipes = array_map( - fn (string|DataPipe $pipe) => is_string($pipe) ? app($pipe) : $pipe, + $pipes = array_map( + fn(string|DataPipe $pipe) => is_string($pipe) ? app($pipe) : $pipe, $this->pipes ); - return $this; + return new ResolvedDataPipeline( + $normalizers, + $pipes, + $this->dataConfig->getDataClass($this->classString) + ); } + /** @deprecated */ public function execute(): Collection { - $properties = null; - - foreach ($this->normalizers as $normalizer) { - $properties = $normalizer->normalize($this->value); - - if ($properties !== null) { - break; - } - } - - if ($properties === null) { - throw CannotCreateData::noNormalizerFound($this->classString, $this->value); - } - - $properties = collect($properties); - - $class = $this->dataConfig->getDataClass($this->classString); - - $properties = ($class->name)::prepareForPipeline($properties); - - foreach ($this->pipes as $pipe) { - $piped = $pipe->handle($this->value, $class, $properties); - - $properties = $piped; - } - - return $properties; + return $this->dataConfig + ->getResolvedDataPipeline($this->classString) + ->execute($this->value); } } diff --git a/src/Resolvers/DataFromSomethingResolver.php b/src/Resolvers/DataFromSomethingResolver.php index e6e38146..ef5a8862 100644 --- a/src/Resolvers/DataFromSomethingResolver.php +++ b/src/Resolvers/DataFromSomethingResolver.php @@ -41,10 +41,10 @@ public function execute(string $class, mixed ...$payloads): BaseData $properties = new Collection(); - $pipeline = $this->dataConfig->getDataPipeLine($class); + $pipeline = $this->dataConfig->getResolvedDataPipeline($class); foreach ($payloads as $payload) { - foreach ($pipeline->using($payload)->execute() as $key => $value) { + foreach ($pipeline->execute($payload) as $key => $value) { $properties[$key] = $value; } } @@ -81,14 +81,11 @@ protected function createFromCustomCreationMethod(string $class, array $payloads return null; } + $pipeline = $this->dataConfig->getResolvedDataPipeline($class); + foreach ($payloads as $payload) { if ($payload instanceof Request) { - $class::pipeline() - ->normalizer(ArrayableNormalizer::class) - ->into($class) - ->using($payload) - ->prepare() - ->execute(); + $pipeline->execute($payload); } } diff --git a/src/Support/DataConfig.php b/src/Support/DataConfig.php index 587bfe39..150317b5 100644 --- a/src/Support/DataConfig.php +++ b/src/Support/DataConfig.php @@ -19,8 +19,8 @@ class DataConfig /** @var array */ protected array $casts = []; - /** @var array */ - protected array $dataPipelines = []; + /** @var array */ + protected array $resolvedDataPipelines = []; /** @var \Spatie\LaravelData\RuleInferrers\RuleInferrer[] */ protected array $ruleInferrers; @@ -50,22 +50,13 @@ public function getDataClass(string $class): DataClass return $this->dataClasses[$class] = DataClass::create(new ReflectionClass($class)); } - public function getDataPipeLine(string $class): DataPipeline + public function getResolvedDataPipeline(string $class): ResolvedDataPipeline { - if(array_key_exists($class, $this->dataPipelines)){ - return $this->dataPipelines[$class]; + if(array_key_exists($class, $this->resolvedDataPipelines)){ + return $this->resolvedDataPipelines[$class]; } - /** @var BaseData $class */ - $pipeline = $class::pipeline(); - - foreach ($class::normalizers() as $normalizer) { - $pipeline->normalizer($normalizer); - } - - $pipeline->prepare(); - - return $this->dataPipelines[$class] = $pipeline; + return $this->resolvedDataPipelines[$class] = $class::pipeline()->resolve(); } public function findGlobalCastForProperty(DataProperty $property): ?Cast @@ -104,4 +95,12 @@ public function getRuleInferrers(): array { return $this->ruleInferrers; } + + public function reset(): self + { + $this->dataClasses = []; + $this->resolvedDataPipelines = []; + + return $this; + } } diff --git a/src/Support/ResolvedDataPipeline.php b/src/Support/ResolvedDataPipeline.php new file mode 100644 index 00000000..8c1cb1fe --- /dev/null +++ b/src/Support/ResolvedDataPipeline.php @@ -0,0 +1,49 @@ + $normalizers + * @param array<\Spatie\LaravelData\DataPipes\DataPipe> $pipes + */ + public function __construct( + protected array $normalizers, + protected array $pipes, + protected DataClass $dataClass, + ) { + } + + public function execute(mixed $value): Collection + { + $properties = null; + + foreach ($this->normalizers as $normalizer) { + $properties = $normalizer->normalize($value); + + if ($properties !== null) { + break; + } + } + + if ($properties === null) { + throw CannotCreateData::noNormalizerFound($this->dataClass->name, $value); + } + + $properties = collect($properties); + + $properties = ($this->dataClass->name)::prepareForPipeline($properties); + + foreach ($this->pipes as $pipe) { + $piped = $pipe->handle($value, $this->dataClass, $properties); + + $properties = $piped; + } + + return $properties; + } +} diff --git a/tests/TestSupport/DataValidationAsserter.php b/tests/TestSupport/DataValidationAsserter.php index c1669940..15270611 100644 --- a/tests/TestSupport/DataValidationAsserter.php +++ b/tests/TestSupport/DataValidationAsserter.php @@ -158,13 +158,12 @@ public function assertAttributes( private function pipePayload(array $payload): array { $properties = app(DataPipeline::class) - ->using($payload) ->normalizer(ArrayNormalizer::class) ->into($this->dataClass) ->through(MapPropertiesDataPipe::class) ->through(ValidatePropertiesDataPipe::class) - ->prepare() - ->execute(); + ->resolve() + ->execute($payload); return $properties->all(); } diff --git a/tests/ValidationTest.php b/tests/ValidationTest.php index ad4bb14e..212ff027 100644 --- a/tests/ValidationTest.php +++ b/tests/ValidationTest.php @@ -13,6 +13,7 @@ use Illuminate\Validation\ValidationException; use Illuminate\Validation\Validator; +use Spatie\LaravelData\Support\DataConfig; use function Pest\Laravel\mock; use function PHPUnit\Framework\assertFalse; @@ -2002,6 +2003,8 @@ public static function pipeline(): DataPipeline expect($data)->toBeInstanceOf(Data::class) ->string->toEqual('nowp'); + app(DataConfig::class)->reset(); + $dataClass::$validateAllTypes = true; $data = $dataClass::from([ From 2e8ca376b5280a393de18c54e1eed04878d89076 Mon Sep 17 00:00:00 2001 From: Ruben Van Assche Date: Fri, 17 Mar 2023 10:23:54 +0100 Subject: [PATCH 4/7] Better benchmarks --- benchmarks/DataBench.php | 54 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/benchmarks/DataBench.php b/benchmarks/DataBench.php index 97ea3f28..51b46a41 100644 --- a/benchmarks/DataBench.php +++ b/benchmarks/DataBench.php @@ -1,14 +1,18 @@ ['simple' => 'Hello'], @@ -41,7 +45,22 @@ public function benchData() } #[Revs(500), Iterations(2)] - public function benchDataCollection() + public function benchDataTransformation() + { + $data = new MultiNestedData( + new NestedData(new SimpleData('Hello')), + new DataCollection(NestedData::class, [ + new NestedData(new SimpleData('I')), + new NestedData(new SimpleData('am')), + new NestedData(new SimpleData('groot')), + ]) + ); + + $data->toArray(); + } + + #[Revs(500), Iterations(2)] + public function benchDataCollectionCreation() { $collection = Collection::times( 15, @@ -71,4 +90,35 @@ public function benchDataCollection() ComplicatedData::collection($collection); } + + #[Revs(500), Iterations(2)] + public function benchDataCollectionTransformation() + { + $collection = Collection::times( + 15, + fn() => new ComplicatedData( + 42, + 42, + true, + 3.14, + 'Hello World', + [1, 1, 2, 3, 5, 8], + null, + Optional::create(), + 42, + CarbonImmutable::create(1994,05,16), + new DateTime('1994-05-16T12:00:00+01:00'), + new SimpleData('hello'), + new DataCollection(NestedData::class, [ + new NestedData(new SimpleData('I')), + new NestedData(new SimpleData('am')), + new NestedData(new SimpleData('groot')), + ]) + ) + )->all(); + + $collection = ComplicatedData::collection($collection); + + $collection->toArray(); + } } From 62774cee3b600fa7b6021470989c5d6a898893eb Mon Sep 17 00:00:00 2001 From: Ruben Van Assche Date: Fri, 17 Mar 2023 10:26:07 +0100 Subject: [PATCH 5/7] Add GH action --- .github/workflows/benchmark.yml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .github/workflows/benchmark.yml diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml new file mode 100644 index 00000000..ce3e95a7 --- /dev/null +++ b/.github/workflows/benchmark.yml @@ -0,0 +1,30 @@ +name: PHPStan + +on: + push: + paths: + - '**.php' + - 'phpbench.json' + pull_request: + paths: + - '**.php' + - 'phpbench.json' + +jobs: + phpstan: + name: phpstan + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '8.2' + coverage: none + + - name: Install composer dependencies + uses: ramsey/composer-install@v2 + + - name: Run Benchmark + run: composer benchmark From e68b7d6ddaa48d59b52a1248256390c77c48aa1d Mon Sep 17 00:00:00 2001 From: Ruben Van Assche Date: Fri, 17 Mar 2023 10:46:20 +0100 Subject: [PATCH 6/7] Better performance --- .github/workflows/benchmark.yml | 2 +- .../DataCollectableTransformer.php | 35 +++++++++++-------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index ce3e95a7..2c94ab4d 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -1,4 +1,4 @@ -name: PHPStan +name: Benchmarks on: push: diff --git a/src/Transformers/DataCollectableTransformer.php b/src/Transformers/DataCollectableTransformer.php index 7a27b3c5..3a19679f 100644 --- a/src/Transformers/DataCollectableTransformer.php +++ b/src/Transformers/DataCollectableTransformer.php @@ -44,27 +44,34 @@ public function transform(): array protected function transformCollection(Enumerable $items): array { - $items = $items->map($this->transformItemClosure()) - ->when( + $payload = []; + + foreach ($items as $key => $item) { + $normalized = $this->transformItemClosure()($item); + + if (! $this->transformValues) { + $payload[$key] = $normalized; + + continue; + } + + $payload[$key] = $normalized->transform( $this->transformValues, - fn (Enumerable $collection) => $collection->map(fn (TransformableData $data) => $data->transform( - $this->transformValues, - $this->wrapExecutionType->shouldExecute() - ? WrapExecutionType::TemporarilyDisabled - : $this->wrapExecutionType, - $this->mapPropertyNames, - )) - ) - ->all(); + $this->wrapExecutionType->shouldExecute() + ? WrapExecutionType::TemporarilyDisabled + : $this->wrapExecutionType, + $this->mapPropertyNames, + ); + } return $this->wrapExecutionType->shouldExecute() - ? $this->wrap->wrap($items) - : $items; + ? $this->wrap->wrap($payload) + : $payload; } protected function transformItemClosure(): Closure { - return fn (BaseData $item) => $item instanceof IncludeableData + return fn(BaseData $item) => $item instanceof IncludeableData ? $item->withPartialTrees($this->trees) : $item; } From 9465cb80fd8510293539a0ccc8b21f21342e6e98 Mon Sep 17 00:00:00 2001 From: rubenvanassche Date: Fri, 17 Mar 2023 09:47:02 +0000 Subject: [PATCH 7/7] Fix styling --- src/DataPipeline.php | 5 ++--- src/Resolvers/DataFromSomethingResolver.php | 3 +-- src/Resolvers/EmptyDataResolver.php | 2 +- src/Support/DataConfig.php | 14 ++++++-------- src/Transformers/DataCollectableTransformer.php | 3 +-- src/Transformers/DataTransformer.php | 2 +- tests/ValidationTest.php | 9 +++++---- 7 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/DataPipeline.php b/src/DataPipeline.php index e8e3e416..114ad439 100644 --- a/src/DataPipeline.php +++ b/src/DataPipeline.php @@ -4,7 +4,6 @@ use Illuminate\Support\Collection; use Spatie\LaravelData\DataPipes\DataPipe; -use Spatie\LaravelData\Exceptions\CannotCreateData; use Spatie\LaravelData\Normalizers\Normalizer; use Spatie\LaravelData\Support\DataConfig; use Spatie\LaravelData\Support\ResolvedDataPipeline; @@ -65,13 +64,13 @@ public function resolve(): ResolvedDataPipeline /** @var \Spatie\LaravelData\Normalizers\Normalizer[] $normalizers */ $normalizers = array_map( - fn(string|Normalizer $normalizer) => is_string($normalizer) ? app($normalizer) : $normalizer, + fn (string|Normalizer $normalizer) => is_string($normalizer) ? app($normalizer) : $normalizer, $normalizers ); /** @var \Spatie\LaravelData\DataPipes\DataPipe[] $pipes */ $pipes = array_map( - fn(string|DataPipe $pipe) => is_string($pipe) ? app($pipe) : $pipe, + fn (string|DataPipe $pipe) => is_string($pipe) ? app($pipe) : $pipe, $this->pipes ); diff --git a/src/Resolvers/DataFromSomethingResolver.php b/src/Resolvers/DataFromSomethingResolver.php index ef5a8862..7eba218e 100644 --- a/src/Resolvers/DataFromSomethingResolver.php +++ b/src/Resolvers/DataFromSomethingResolver.php @@ -5,7 +5,6 @@ use Illuminate\Http\Request; use Illuminate\Support\Collection; use Spatie\LaravelData\Contracts\BaseData; -use Spatie\LaravelData\Normalizers\ArrayableNormalizer; use Spatie\LaravelData\Support\DataConfig; use Spatie\LaravelData\Support\DataMethod; @@ -63,7 +62,7 @@ protected function createFromCustomCreationMethod(string $class, array $payloads ->getDataClass($class) ->methods ->filter( - fn(DataMethod $method) => $method->isCustomCreationMethod + fn (DataMethod $method) => $method->isCustomCreationMethod && ! in_array($method->name, $this->ignoredMagicalMethods) ); diff --git a/src/Resolvers/EmptyDataResolver.php b/src/Resolvers/EmptyDataResolver.php index 9bab4539..9c86e2e5 100644 --- a/src/Resolvers/EmptyDataResolver.php +++ b/src/Resolvers/EmptyDataResolver.php @@ -19,7 +19,7 @@ public function execute(string $class, array $extra = []): array $payload = []; - foreach ($dataClass->properties as $property){ + foreach ($dataClass->properties as $property) { $name = $property->outputMappedName ?? $property->name; if ($property->hasDefaultValue) { diff --git a/src/Support/DataConfig.php b/src/Support/DataConfig.php index 150317b5..c9591b5e 100644 --- a/src/Support/DataConfig.php +++ b/src/Support/DataConfig.php @@ -4,8 +4,6 @@ use ReflectionClass; use Spatie\LaravelData\Casts\Cast; -use Spatie\LaravelData\Contracts\BaseData; -use Spatie\LaravelData\DataPipeline; use Spatie\LaravelData\Transformers\Transformer; class DataConfig @@ -19,7 +17,7 @@ class DataConfig /** @var array */ protected array $casts = []; - /** @var array */ + /** @var array */ protected array $resolvedDataPipelines = []; /** @var \Spatie\LaravelData\RuleInferrers\RuleInferrer[] */ @@ -50,9 +48,9 @@ public function getDataClass(string $class): DataClass return $this->dataClasses[$class] = DataClass::create(new ReflectionClass($class)); } - public function getResolvedDataPipeline(string $class): ResolvedDataPipeline + public function getResolvedDataPipeline(string $class): ResolvedDataPipeline { - if(array_key_exists($class, $this->resolvedDataPipelines)){ + if (array_key_exists($class, $this->resolvedDataPipelines)) { return $this->resolvedDataPipelines[$class]; } @@ -98,9 +96,9 @@ public function getRuleInferrers(): array public function reset(): self { - $this->dataClasses = []; - $this->resolvedDataPipelines = []; + $this->dataClasses = []; + $this->resolvedDataPipelines = []; - return $this; + return $this; } } diff --git a/src/Transformers/DataCollectableTransformer.php b/src/Transformers/DataCollectableTransformer.php index 3a19679f..5c3bdbb4 100644 --- a/src/Transformers/DataCollectableTransformer.php +++ b/src/Transformers/DataCollectableTransformer.php @@ -9,7 +9,6 @@ use Illuminate\Support\Enumerable; use Spatie\LaravelData\Contracts\BaseData; use Spatie\LaravelData\Contracts\IncludeableData; -use Spatie\LaravelData\Contracts\TransformableData; use Spatie\LaravelData\Support\PartialTrees; use Spatie\LaravelData\Support\Wrapping\Wrap; use Spatie\LaravelData\Support\Wrapping\WrapExecutionType; @@ -71,7 +70,7 @@ protected function transformCollection(Enumerable $items): array protected function transformItemClosure(): Closure { - return fn(BaseData $item) => $item instanceof IncludeableData + return fn (BaseData $item) => $item instanceof IncludeableData ? $item->withPartialTrees($this->trees) : $item; } diff --git a/src/Transformers/DataTransformer.php b/src/Transformers/DataTransformer.php index 63f1cf35..736f2b09 100644 --- a/src/Transformers/DataTransformer.php +++ b/src/Transformers/DataTransformer.php @@ -230,7 +230,7 @@ protected function resolvePropertyValue( }; if ($value instanceof TransformableData && $this->transformValues) { - return $value->transform($this->transformValues, $wrapExecutionType, $this->mapPropertyNames,); + return $value->transform($this->transformValues, $wrapExecutionType, $this->mapPropertyNames, ); } return $value; diff --git a/tests/ValidationTest.php b/tests/ValidationTest.php index 212ff027..93fd8f74 100644 --- a/tests/ValidationTest.php +++ b/tests/ValidationTest.php @@ -13,21 +13,21 @@ use Illuminate\Validation\ValidationException; use Illuminate\Validation\Validator; -use Spatie\LaravelData\Support\DataConfig; use function Pest\Laravel\mock; use function PHPUnit\Framework\assertFalse; use Spatie\LaravelData\Attributes\DataCollectionOf; use Spatie\LaravelData\Attributes\MapInputName; -use Spatie\LaravelData\Attributes\MapName; +use Spatie\LaravelData\Attributes\MapName; use Spatie\LaravelData\Attributes\Validation\ArrayType; use Spatie\LaravelData\Attributes\Validation\Bail; -use Spatie\LaravelData\Attributes\Validation\Exists; +use Spatie\LaravelData\Attributes\Validation\Exists; use Spatie\LaravelData\Attributes\Validation\In; + use Spatie\LaravelData\Attributes\Validation\IntegerType; use Spatie\LaravelData\Attributes\Validation\Max; use Spatie\LaravelData\Attributes\Validation\Min; @@ -40,8 +40,8 @@ use Spatie\LaravelData\Attributes\WithoutValidation; use Spatie\LaravelData\Data; use Spatie\LaravelData\DataCollection; - use Spatie\LaravelData\DataPipeline; + use Spatie\LaravelData\DataPipes\AuthorizedDataPipe; use Spatie\LaravelData\DataPipes\CastPropertiesDataPipe; use Spatie\LaravelData\DataPipes\DefaultValuesDataPipe; @@ -53,6 +53,7 @@ use Spatie\LaravelData\Normalizers\ModelNormalizer; use Spatie\LaravelData\Normalizers\ObjectNormalizer; use Spatie\LaravelData\Optional; +use Spatie\LaravelData\Support\DataConfig; use Spatie\LaravelData\Support\Validation\References\FieldReference; use Spatie\LaravelData\Support\Validation\References\RouteParameterReference; use Spatie\LaravelData\Support\Validation\ValidationContext;