From 825d5c333adfbd2012549262c6abd290f40a3e4e Mon Sep 17 00:00:00 2001 From: Marvin Petker Date: Sat, 7 Feb 2026 03:34:33 +0100 Subject: [PATCH 1/6] refactor: unify hydration and validation flow with circular reference detection Reworked DTO hydration and validation to rely fully on Symfony Validator and a centralized `RequestDtoFactory`. - Introduce a new `RequestDtoFactory` to handle recursive DTO instantiation - Detect circular DTO references and throw `CircularReferenceException` - Streamline hydration/validation flow and merge hydration violations correctly - Remove custom `RequestDtoValidator` and `GroupSequenceExtractor` - Integrate `QueryParam` type transformation into the factory - Fix duplicate violation registration caused by name collisions - Improve nested DTO handling and null-safety - Remove `RequestDtoTrait` and update related tests - Update unit and integration tests to reflect the new architecture Refs: `phpstan`, `cs-fixer` BREAKING-CHANGE: Validation and hydration flow has changed; `RequestDtoTrait` was removed --- config/services.php | 19 +- config/services_test.php | 1 + src/Attribute/AbstractNestedParam.php | 2 +- .../RequestDtoValidationEventSubscriber.php | 21 +- .../Exception/CircularReferenceException.php | 36 ++ .../Exception/PropertyHydrationException.php | 35 ++ .../RequestDtoHydrationException.php | 39 ++ src/Factory/RequestDtoFactory.php | 361 ++++++++++++++++++ src/Reflection/RequestDtoMetadata.php | 13 +- src/Reflection/RequestDtoMetadataFactory.php | 5 + .../RequestDtoParamMetadataFactory.php | 6 +- src/RequestDtoResolver.php | 16 +- src/Trait/RequestDtoTrait.php | 63 --- src/Utility/DtoInstanceBag.php | 20 + src/Utility/DtoInstanceBagInterface.php | 27 ++ src/Utility/DtoReflectionHelper.php | 2 + src/Utility/TypeErrorInfo.php | 2 +- src/Validator/GroupSequenceExtractor.php | 76 ---- src/Validator/RequestDtoValidator.php | 313 --------------- test/Fixtures/CircularReferencingDto.php | 30 ++ test/Fixtures/DeepNestedDto.php | 3 + test/Fixtures/DtoWithChildDto.php | 3 + .../Fixtures/DtoWithGroupSequenceProvider.php | 2 - test/Fixtures/DtoWithNestedDtoArray.php | 2 + .../GroupProvider/TestGroupProvider.php | 2 +- test/Fixtures/GroupSequenceProviderDTO.php | 5 +- test/Fixtures/StrictTypesDto.php | 4 +- .../RequestDtoFactoryIntegrationTest.php | 182 +++++++++ ...equestDtoResolverBundleIntegrationTest.php | 49 ++- .../RequestDtoValidatorIntegrationTest.php | 300 --------------- ...equestDtoValidationEventSubscriberTest.php | 55 ++- .../RequestDtoMetadataFactoryTest.php | 2 +- .../RequestDtoParamMetadataTest.php | 4 +- test/Unit/RequestDtoResolverTest.php | 78 ++-- test/Unit/Trait/RequestDtoTraitTest.php | 128 ------- test/Unit/Utility/DtoInstanceBagTest.php | 21 + test/Unit/Utility/DtoReflectionHelperTest.php | 4 + .../Validator/GroupSequenceExtractorTest.php | 175 --------- 38 files changed, 936 insertions(+), 1170 deletions(-) create mode 100644 src/Factory/Exception/CircularReferenceException.php create mode 100644 src/Factory/Exception/PropertyHydrationException.php create mode 100644 src/Factory/Exception/RequestDtoHydrationException.php create mode 100644 src/Factory/RequestDtoFactory.php delete mode 100644 src/Trait/RequestDtoTrait.php delete mode 100644 src/Validator/GroupSequenceExtractor.php delete mode 100644 src/Validator/RequestDtoValidator.php create mode 100644 test/Fixtures/CircularReferencingDto.php create mode 100644 test/Integration/RequestDtoFactoryIntegrationTest.php delete mode 100644 test/Integration/RequestDtoValidatorIntegrationTest.php delete mode 100644 test/Unit/Trait/RequestDtoTraitTest.php delete mode 100644 test/Unit/Validator/GroupSequenceExtractorTest.php diff --git a/config/services.php b/config/services.php index 9752acc..ce67b0b 100644 --- a/config/services.php +++ b/config/services.php @@ -15,6 +15,7 @@ use Crtl\RequestDtoResolverBundle\EventSubscriber\RequestDtoValidationEventSubscriber; use Crtl\RequestDtoResolverBundle\EventSubscriber\RequestValidationExceptionEventSubscriber; +use Crtl\RequestDtoResolverBundle\Factory\RequestDtoFactory; use Crtl\RequestDtoResolverBundle\PropertyInfo\PropertyInfoExtractorFactory; use Crtl\RequestDtoResolverBundle\Reflection\RequestDtoMetadataFactory; use Crtl\RequestDtoResolverBundle\Reflection\RequestDtoParamMetadataFactory; @@ -22,8 +23,6 @@ use Crtl\RequestDtoResolverBundle\Utility\DtoInstanceBag; use Crtl\RequestDtoResolverBundle\Utility\DtoInstanceBagInterface; use Crtl\RequestDtoResolverBundle\Utility\DtoReflectionHelper; -use Crtl\RequestDtoResolverBundle\Validator\GroupSequenceExtractor; -use Crtl\RequestDtoResolverBundle\Validator\RequestDtoValidator; use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor; use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor; use Symfony\Component\PropertyInfo\PropertyInfoExtractorInterface; @@ -43,12 +42,6 @@ $services->alias(DtoInstanceBagInterface::class, DtoInstanceBag::class); - $services->set(GroupSequenceExtractor::class) - ->args([ - '$groupProviderLocator' => service('service_container'), - ]) - ->private(); - $services->set(ReflectionExtractor::class) ->private(); @@ -84,25 +77,23 @@ '$cache' => service('cache.system'), ]); - $services->set(RequestDtoValidator::class) + $services->set(RequestDtoFactory::class) ->args([ - '$validator' => service('validator'), '$metadataFactory' => service(RequestDtoMetadataFactory::class), - '$groupSequenceExtractor' => service(GroupSequenceExtractor::class), ]) - ->public(); + ->public(); $services->set(RequestDtoResolver::class) ->args([ '$dtoInstanceBag' => service(DtoInstanceBagInterface::class), - '$factory' => service(RequestDtoMetadataFactory::class), '$reflectionHelper' => service(DtoReflectionHelper::class), + '$requestDtoFactory' => service(RequestDtoFactory::class), ]) ->tag('controller.argument_value_resolver', ['priority' => 50]); $services->set(RequestDtoValidationEventSubscriber::class) ->args([ - '$validator' => service(RequestDtoValidator::class), + '$validator' => service('validator'), '$dtoInstanceBag' => service(DtoInstanceBagInterface::class), ]) ->tag('kernel.event_subscriber'); diff --git a/config/services_test.php b/config/services_test.php index 547705e..ad38f92 100644 --- a/config/services_test.php +++ b/config/services_test.php @@ -21,5 +21,6 @@ ->public(); $services->set(Crtl\RequestDtoResolverBundle\Test\Fixtures\GroupProvider\TestGroupProvider::class) + ->tag('validator.group_provider') ->public(); }; diff --git a/src/Attribute/AbstractNestedParam.php b/src/Attribute/AbstractNestedParam.php index ecf21e5..d9dd81a 100644 --- a/src/Attribute/AbstractNestedParam.php +++ b/src/Attribute/AbstractNestedParam.php @@ -53,7 +53,7 @@ public function getValueFromRequest(Request $request): mixed $data = $this->getDataFromRequest($request); } - $defaultValue = null; // $this->property?->getDefaultValue(); + $defaultValue = $this->property?->getDefaultValue(); $value = $data[$name] ?? $defaultValue; diff --git a/src/EventSubscriber/RequestDtoValidationEventSubscriber.php b/src/EventSubscriber/RequestDtoValidationEventSubscriber.php index 40bea1a..a97e1a5 100644 --- a/src/EventSubscriber/RequestDtoValidationEventSubscriber.php +++ b/src/EventSubscriber/RequestDtoValidationEventSubscriber.php @@ -14,20 +14,19 @@ namespace Crtl\RequestDtoResolverBundle\EventSubscriber; use Crtl\RequestDtoResolverBundle\Exception\RequestValidationException; -use Crtl\RequestDtoResolverBundle\RequestDtoResolver; use Crtl\RequestDtoResolverBundle\Utility\DtoInstanceBagInterface; -use Crtl\RequestDtoResolverBundle\Validator\RequestDtoValidator; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent; use Symfony\Component\HttpKernel\KernelEvents; +use Symfony\Component\Validator\Validator\ValidatorInterface; /** - * Event subscriber which validates DTOs that have been resolved before by {@link RequestDtoResolver}. + * Event subscriber which validates DTOs that have been resolved before by {@link \Crtl\RequestDtoResolverBundle\RequestDtoResolver}. */ final class RequestDtoValidationEventSubscriber implements EventSubscriberInterface { public function __construct( - private RequestDtoValidator $validator, + private ValidatorInterface $validator, private DtoInstanceBagInterface $dtoInstanceBag, ) { } @@ -46,10 +45,18 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event): vo { $request = $event->getRequest(); - foreach ($this->dtoInstanceBag->getRegisteredInstances($request) as $instance) { - $violations = $this->validator->validateAndHydrate($instance, $request); + foreach ($this->dtoInstanceBag->getRegisteredInstances($request) as $className => $instance) { + // Check for stored hydration violations first + $hydrationViolations = $this->dtoInstanceBag->getHydrationViolations($className, $request); - if ($violations->count()) { + // Standard Symfony validation on fully-hydrated DTO + $violations = $this->validator->validate($instance); + + if (null !== $hydrationViolations) { + $violations->addAll($hydrationViolations); + } + + if ($violations->count() > 0) { throw new RequestValidationException($instance, $violations); } } diff --git a/src/Factory/Exception/CircularReferenceException.php b/src/Factory/Exception/CircularReferenceException.php new file mode 100644 index 0000000..eee5b6d --- /dev/null +++ b/src/Factory/Exception/CircularReferenceException.php @@ -0,0 +1,36 @@ + %s', + $className, + implode(' -> ', $stack), + $className, + )); + } +} diff --git a/src/Factory/Exception/PropertyHydrationException.php b/src/Factory/Exception/PropertyHydrationException.php new file mode 100644 index 0000000..26fec10 --- /dev/null +++ b/src/Factory/Exception/PropertyHydrationException.php @@ -0,0 +1,35 @@ +object)); + parent::__construct($message, previous: $previous); + } +} diff --git a/src/Factory/RequestDtoFactory.php b/src/Factory/RequestDtoFactory.php new file mode 100644 index 0000000..c84fdb3 --- /dev/null +++ b/src/Factory/RequestDtoFactory.php @@ -0,0 +1,361 @@ + 42, // ✅ works + * "age_param" => 42 // ❌ ignored + * ] + * + * @template TObject of object + * + * @param class-string $className fully-qualified DTO class name + * @param array $data input data keyed by DTO property name + * + * @return TObject hydrated DTO instance + * + * @throws RequestDtoHydrationException on type errors or nested hydration errors + * @throws CircularReferenceException on circular reference during nested instantiation + * @throws \ReflectionException on reflection/metadata failures + */ + public function fromArray(string $className, array $data): object + { + return $this->createInstanceRecursive( + $className, + $data, + fn (AbstractParam $attr, RequestDtoParamMetadata $paramMetadata, array $data) => $data[$paramMetadata->getPropertyName()] ?? null, + ); + } + + /** + * Creates and hydrates a Request DTO from an HTTP request. + * + * Values are resolved using the parameter attributes attached to each + * DTO property (e.g. {@see QueryParam}, {@see BodyParam}, {@see RouteParam}, {@see FileParam}). + * + * @template TObject of object + * + * @param class-string $className fully-qualified DTO class name + * @param Request $request HTTP request used as value source + * + * @return TObject hydrated DTO instance + * + * @throws RequestDtoHydrationException on type errors or nested hydration errors + * @throws CircularReferenceException on circular reference during nested instantiation + * @throws \ReflectionException on reflection/metadata failures + */ + public function fromRequest(string $className, Request $request): object + { + return $this->createInstanceRecursive( + $className, + $request, + fn (AbstractParam $attr, RequestDtoParamMetadata $paramMetadata, Request $request) => $attr->getValueFromRequest($request), + ); + } + + /** + * Internal helper used to either create DTO from request or array. + * + * @template TObject of object + * @template TContext of (Request|array) + * + * @param class-string $className DTO class name to instantiate + * @param TContext $context data source used to resolve values + * @param callable(AbstractParam, RequestDtoParamMetadata, TContext): mixed $valueProvider resolves a raw value for a single property + * @param AbstractParam|null $parentAttribute parent attribute for nested hydration + * @param RequestDtoMetadata|null $metadata pre-resolved metadata for recursion reuse + * @param string[] $stack DTO class stack used for circular reference detection + * + * @return TObject hydrated DTO instance + * + * @throws \ReflectionException on reflection/metadata failures + * @throws RequestDtoHydrationException on type errors or nested hydration errors + * @throws CircularReferenceException on circular reference during nested instantiation + */ + private function createInstanceRecursive( + string $className, + Request|array $context, + callable $valueProvider, + ?AbstractParam $parentAttribute = null, + ?RequestDtoMetadata $metadata = null, + array $stack = [], + ): object { + if (in_array($className, $stack, true)) { + throw new CircularReferenceException($className, $stack); + } + + $stack[] = $className; + + $metadata ??= $this->metadataFactory->getMetadataFor($className); + + $newInstanceArgs = $context instanceof Request ? [$context] : []; + + /** @var TObject $object */ + $object = $metadata->newInstance(...$newInstanceArgs); + + $violations = new ConstraintViolationList(); + + foreach ($metadata->getPropertyMetadataGenerator() as $propertyMetadata) { + $propertyName = $propertyMetadata->getPropertyName(); + + $nestedClassName = $propertyMetadata->getNestedDtoClassName(); + + $attr = $propertyMetadata->getAttribute($parentAttribute); + + if ($attr instanceof QueryParam) { + $builtInType = strtolower($propertyMetadata->getBuiltinType()); + if (!$attr->hasTransformType() && QueryParam::isTransformType($builtInType)) { + $attr->setTransformType($builtInType); + } + } + + $requestValue = $valueProvider($attr, $propertyMetadata, $context); + + $value = $requestValue + ?? $propertyMetadata->getDefaultValue(); + + // Property is typed with nested dto + if (null !== $nestedClassName && null !== $value) { + $isArray = $propertyMetadata->isNestedDtoArray(); + + /** @var class-string $nestedClassName */ + $nestedMetadata = $this->metadataFactory + ->getMetadataFor($nestedClassName); + + $valueArray = $isArray ? array_values($value) : [$value]; + $nestedViolations = new ConstraintViolationList(); + + $resultArray = []; + foreach ($valueArray as $i => $nestedValue) { + /** @var AbstractParam $nestedAttr */ + $nestedAttr = clone $attr; + if ($isArray && $nestedAttr instanceof AbstractNestedParam) { + $nestedAttr->setIndex($i); + } + + try { + $resultArray[] = $this->createInstanceRecursive( + $nestedClassName, + $context instanceof Request + ? $context + : $nestedValue, + $valueProvider, + $nestedAttr, + $nestedMetadata, + $stack, + ); + } catch (RequestDtoHydrationException $e) { + $nestedValueViolations = $this->prefixViolations( + $e->violations, + // Append index to prefix only when in array mode + $isArray ? ($propertyName."[$i]") : $propertyName, + ); + + // Collect all nested violations first before exiting outside of loop to ensure all nested items are validated. + $nestedViolations->addAll($nestedValueViolations); + } + } + + if ($nestedViolations->count() > 0) { + $violations->addAll($nestedViolations); + continue; // continue with next property + } + + $value = $isArray ? $resultArray : $resultArray[0]; + } + + try { + $this->assignProperty($object, $propertyMetadata, $value); + } catch (PropertyHydrationException $e) { + $violation = $this->createTypeErrorViolation( + $e->typeError, + $object, + $propertyMetadata, + $value, + ); + $violations->add($violation); + } + } + + if ($violations->count() > 0) { + throw new RequestDtoHydrationException($object, $violations); + } + + return $object; + } + + /** + * Helper to assign value to property and handle any occurring {@link \TypeError} by transforming them into {@link ConstraintViolation}. + * + * @throws PropertyHydrationException When the property could not be assigned + */ + private function assignProperty( + object $object, + RequestDtoParamMetadata $metadata, + mixed $value, + ): void { + try { + $propertyName = $metadata->getPropertyName(); + $object->$propertyName = $value; + } catch (\TypeError $e) { + throw new PropertyHydrationException(get_class($object), $metadata->getPropertyName(), $value, $e); + } + } + + /** + * Creates custom constraint violation for type errors occuring when assigning values to types properties and the type is not matching. + * + * @param \TypeError $error The type error that was thrown + * @param object $object The object being validated + * @param RequestDtoParamMetadata $propertyMetadata The metadata of the property being validated + */ + private function createTypeErrorViolation( + \TypeError $error, + object $object, + RequestDtoParamMetadata $propertyMetadata, + mixed $invalidValue = null + ): ConstraintViolation { + $errorInfo = new TypeErrorInfo($error); + + $template = 'This value should be of type {{ expected }}, {{ given }} given.1'; + + $params = [ + '{{ expected }}' => $errorInfo->expectedType, + '{{ given }}' => $errorInfo->actualType, + ]; + + $message = $this->translator?->trans($template, $params, 'validators') + ?? str_replace(array_keys($params), array_values($params), $template); + + return new ConstraintViolation( + $message, + $template, + $params, + $object, + $propertyMetadata->getPropertyName(), + $invalidValue, + ); + } + + /** + * Prefixes property paths of constraint violations with parent property name. + */ + private function prefixViolations(ConstraintViolationListInterface $violations, string $prefix): ConstraintViolationList + { + $prefixedList = new ConstraintViolationList(); + foreach ($violations as $violation) { + $prefixedList->add($this->prefixViolation($violation, $prefix)); + } + + return $prefixedList; + } + + private function prefixViolation(ConstraintViolationInterface $violation, string $prefix): ConstraintViolation + { + $path = $prefix; + $violationPath = $violation->getPropertyPath(); + + if (!empty($violationPath) && !str_starts_with($violationPath, '[')) { + $violationPath = '.'.$violationPath; + } + + $path .= $violationPath; + + return new ConstraintViolation( + $violation->getMessage(), + $violation->getMessageTemplate(), + $violation->getParameters(), + $violation->getRoot(), + $path, + $violation->getInvalidValue(), + $violation->getPlural(), + $violation->getCode(), + $violation->getConstraint(), + $violation->getCause(), + ); + } +} diff --git a/src/Reflection/RequestDtoMetadata.php b/src/Reflection/RequestDtoMetadata.php index cbb65f9..757d622 100644 --- a/src/Reflection/RequestDtoMetadata.php +++ b/src/Reflection/RequestDtoMetadata.php @@ -54,11 +54,12 @@ public function __construct( */ private readonly ClassMetadataInterface $validatorMetadata, ) { - foreach ($propertyMetadata as $propertyMetadata) { - $this->propertyMetadata[$propertyMetadata->getPropertyName()] = $propertyMetadata; + foreach ($propertyMetadata as $propMetadata) { + $propertyName = $propMetadata->getPropertyName(); + $this->propertyMetadata[$propertyName] = $propMetadata; - if ($propertyMetadata->isConstrained()) { - $this->constrainedProperties[$propertyMetadata->getPropertyName()] = $propertyMetadata; + if ($propMetadata->isConstrained()) { + $this->constrainedProperties[$propertyName] = $propMetadata; } } } @@ -172,12 +173,12 @@ public function isConstrainedProperty(string|\ReflectionProperty $propertyName): * * @throws \ReflectionException */ - public function newInstance(...$args): ?object + public function newInstance(...$args): object { $class = $this->getReflectionClass(); return $class->getConstructor() - ? $class->newInstanceArgs($args) + ? $class->newInstance(...$args) : $class->newInstanceWithoutConstructor() ; } diff --git a/src/Reflection/RequestDtoMetadataFactory.php b/src/Reflection/RequestDtoMetadataFactory.php index bacc84e..2312e80 100644 --- a/src/Reflection/RequestDtoMetadataFactory.php +++ b/src/Reflection/RequestDtoMetadataFactory.php @@ -42,6 +42,11 @@ public function getMetadataFor(string $className): RequestDtoMetadata } $reflectionClass = new \ReflectionClass($className); + + if (!$reflectionClass->isInstantiable()) { + throw new \LogicException(sprintf('DTO class "%s" must be instantiable.', $reflectionClass->getName())); + } + $validatorMetadata = $this->validator->getMetadataFor($className); assert($validatorMetadata instanceof ClassMetadataInterface, 'Validator metadata for '.$className.' could not be retrieved'); diff --git a/src/Reflection/RequestDtoParamMetadataFactory.php b/src/Reflection/RequestDtoParamMetadataFactory.php index a2a0150..b9e5972 100644 --- a/src/Reflection/RequestDtoParamMetadataFactory.php +++ b/src/Reflection/RequestDtoParamMetadataFactory.php @@ -39,14 +39,14 @@ public function getMetadataFor(\ReflectionProperty $property): RequestDtoParamMe /** @var ClassMetadataInterface $validatorClassMetadata */ $validatorClassMetadata = $this->validator->getMetadataFor($className); - $constraintedProperties = $validatorClassMetadata->getConstrainedProperties(); + $constrainedProperties = $validatorClassMetadata->getConstrainedProperties(); $propertyName = $property->getName(); try { $type = $this->propertyInfoExtractor->getType($className, $propertyName); } catch (\InvalidArgumentException $e) { - // Compabitibility fix because somehow type-info does not support unions with mixed. + // Compatibility fix because somehow type-info does not support unions with mixed. if ('Cannot create union with "mixed" standalone type.' !== $e->getMessage()) { throw $e; } @@ -93,7 +93,7 @@ public function getMetadataFor(\ReflectionProperty $property): RequestDtoParamMe $property->getDeclaringClass()->getName(), $propertyName, $typeDescription['builtInType'], - in_array($propertyName, $constraintedProperties, true), + in_array($propertyName, $constrainedProperties, true), $definetlyClassString, $isArrayType, // check if type is nullable and fall back to true when no type specified diff --git a/src/RequestDtoResolver.php b/src/RequestDtoResolver.php index e2caf5d..c9ed0dc 100644 --- a/src/RequestDtoResolver.php +++ b/src/RequestDtoResolver.php @@ -13,7 +13,8 @@ namespace Crtl\RequestDtoResolverBundle; -use Crtl\RequestDtoResolverBundle\Reflection\RequestDtoMetadataFactory; +use Crtl\RequestDtoResolverBundle\Factory\Exception\RequestDtoHydrationException; +use Crtl\RequestDtoResolverBundle\Factory\RequestDtoFactory; use Crtl\RequestDtoResolverBundle\Utility\DtoInstanceBagInterface; use Crtl\RequestDtoResolverBundle\Utility\DtoReflectionHelper; use Symfony\Component\HttpFoundation\Request; @@ -30,8 +31,8 @@ { public function __construct( private DtoInstanceBagInterface $dtoInstanceBag, - private RequestDtoMetadataFactory $factory, private DtoReflectionHelper $reflectionHelper, + private RequestDtoFactory $requestDtoFactory, ) { } @@ -50,12 +51,11 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable return []; } - /** @var class-string $type */ - $metadata = $this->factory->getMetadataFor($type); - $object = $metadata->newInstance($request); - - if (null === $object) { - throw new \RuntimeException('Failed to instantiate request dto '.$type); + try { + $object = $this->requestDtoFactory->fromRequest($type, $request); + } catch (RequestDtoHydrationException $e) { + $object = $e->object; + $this->dtoInstanceBag->registerHydrationViolations(get_class($object), $e->violations, $request); } $this->dtoInstanceBag->registerInstance($object, $request); diff --git a/src/Trait/RequestDtoTrait.php b/src/Trait/RequestDtoTrait.php deleted file mode 100644 index 62b29cc..0000000 --- a/src/Trait/RequestDtoTrait.php +++ /dev/null @@ -1,63 +0,0 @@ -getProperty($property); - - // Return property when initialized - if ($reflectionProperty->isInitialized($this)) { - $value = $reflectionProperty->getValue($this); - - if ($value !== $reflectionProperty->getDefaultValue()) { - return $value; - } - } - - if (null === $this->request) { - throw new \LogicException('Request must be set before calling getValue().'); - } - - $attrs = $reflectionProperty->getAttributes(AbstractParam::class, \ReflectionAttribute::IS_INSTANCEOF); - - if (empty($attrs)) { - return $this->request->request->get($property); - } - - /** @var AbstractParam $attr */ - $attr = $attrs[0]->newInstance(); - $attr->setProperty($reflectionProperty); - - return $attr->getValueFromRequest($this->request); - } -} diff --git a/src/Utility/DtoInstanceBag.php b/src/Utility/DtoInstanceBag.php index a87b410..42891b8 100644 --- a/src/Utility/DtoInstanceBag.php +++ b/src/Utility/DtoInstanceBag.php @@ -14,6 +14,7 @@ namespace Crtl\RequestDtoResolverBundle\Utility; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Validator\ConstraintViolationListInterface; final class DtoInstanceBag implements DtoInstanceBagInterface { @@ -39,6 +40,25 @@ public function registerInstance(object $instance, Request $request): void )); } + public function registerHydrationViolations( + string $className, + ConstraintViolationListInterface $violations, + Request $request + ): void { + /** @var array $existing */ + $existing = $request->attributes->get(self::DTO_HYDRATION_VIOLATIONS_KEY, []); + $existing[$className] = $violations; + $request->attributes->set(self::DTO_HYDRATION_VIOLATIONS_KEY, $existing); + } + + public function getHydrationViolations(string $className, Request $request): ?ConstraintViolationListInterface + { + /** @var array $violations */ + $violations = $request->attributes->get(self::DTO_HYDRATION_VIOLATIONS_KEY, []); + + return $violations[$className] ?? null; + } + /** * @param class-string $className */ diff --git a/src/Utility/DtoInstanceBagInterface.php b/src/Utility/DtoInstanceBagInterface.php index b9155ea..e8c1faf 100644 --- a/src/Utility/DtoInstanceBagInterface.php +++ b/src/Utility/DtoInstanceBagInterface.php @@ -14,6 +14,7 @@ namespace Crtl\RequestDtoResolverBundle\Utility; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Validator\ConstraintViolationListInterface; interface DtoInstanceBagInterface { @@ -24,6 +25,13 @@ interface DtoInstanceBagInterface */ public const DTO_INSTANCES_ATTRIBUTE_KEY = '_request_dto_instances'; + /** + * Name of the request attribute that stores hydration violations. + * + * @internal + */ + public const DTO_HYDRATION_VIOLATIONS_KEY = '_request_dto_hydration_violations'; + /** * @return array */ @@ -31,6 +39,25 @@ public function getRegisteredInstances(Request $request): array; public function registerInstance(object $instance, Request $request): void; + /** + * Register constraint violation that occured during hydration of DTO. + * + * These constraints are normally violated when using strict typed DTOs + * with mismatching request data. + * + * @param class-string $className + */ + public function registerHydrationViolations( + string $className, + ConstraintViolationListInterface $violations, + Request $request + ): void; + + /** + * @param class-string $className + */ + public function getHydrationViolations(string $className, Request $request): ?ConstraintViolationListInterface; + /** * @param class-string $className */ diff --git a/src/Utility/DtoReflectionHelper.php b/src/Utility/DtoReflectionHelper.php index bf6fa6b..0f29449 100644 --- a/src/Utility/DtoReflectionHelper.php +++ b/src/Utility/DtoReflectionHelper.php @@ -89,6 +89,8 @@ public function getDtoClassNameFromReflectionProperty( * Checks whether given object, relfection class or class name is a request dto. * * @throws \ReflectionException + * + * @phpstan-assert-if-true class-string|object $class */ public function isRequestDto(string|object $class): bool { diff --git a/src/Utility/TypeErrorInfo.php b/src/Utility/TypeErrorInfo.php index 34f58e8..cf2f91f 100644 --- a/src/Utility/TypeErrorInfo.php +++ b/src/Utility/TypeErrorInfo.php @@ -74,7 +74,7 @@ public function __construct( if (self::ERROR_TYPE_PROPERTY === $errorType) { preg_match( - '/Cannot assign (\w+) to property ([^ ]+) of type (\w+)/', + '/Cannot assign ([\w\\\]+) to property ([^ ]+) of type (\w+)/', $message, $m, ); diff --git a/src/Validator/GroupSequenceExtractor.php b/src/Validator/GroupSequenceExtractor.php deleted file mode 100644 index 6407982..0000000 --- a/src/Validator/GroupSequenceExtractor.php +++ /dev/null @@ -1,76 +0,0 @@ - $groups - * - * @return string[]|mixed[]|GroupSequence - * - * @throws \Psr\Container\ContainerExceptionInterface - * @throws \Psr\Container\NotFoundExceptionInterface - */ - public function getGroupSequence(object $object, array $groups, ClassMetadataInterface $metadata): array|GroupSequence - { - $className = $metadata->getClassName(); - - // Return groups when not in default group - if (!in_array($className, $groups) || !in_array(Constraint::DEFAULT_GROUP, $groups)) { - return $groups; - } - - if ($metadata->hasGroupSequence()) { - // The group sequence is statically defined for the class - return $metadata->getGroupSequence(); - } elseif ($metadata->isGroupSequenceProvider()) { - // @phpstan-ignore function.alreadyNarrowedType - if (method_exists($metadata, 'getGroupProvider') && null !== $provider = $metadata->getGroupProvider()) { - if (null === $this->groupProviderLocator) { - throw new \LogicException('A group provider locator is required when using group provider.'); - } - - /** @var GroupProviderInterface $provider */ - $provider = $this->groupProviderLocator->get($provider); - $group = $provider->getGroups($object); - } else { - assert($object instanceof GroupSequenceProviderInterface); - // The group sequence is dynamically obtained from the validated - // object - $group = $object->getGroupSequence(); - } - - if (!$group instanceof GroupSequence) { - $group = new GroupSequence($group); - } - - return $group; - } - - return $groups; - } -} diff --git a/src/Validator/RequestDtoValidator.php b/src/Validator/RequestDtoValidator.php deleted file mode 100644 index ac185bd..0000000 --- a/src/Validator/RequestDtoValidator.php +++ /dev/null @@ -1,313 +0,0 @@ - 0, - 'string' => '', - 'float' => 0.0, - 'bool' => false, - 'array' => [], - 'object' => null, - 'null' => null, - 'mixed' => null, - 'iterable' => [], - ]; - - public function __construct( - private readonly ValidatorInterface $validator, - private readonly RequestDtoMetadataFactory $metadataFactory, - private readonly GroupSequenceExtractor $groupSequenceExtractor, - private readonly ?TranslatorInterface $translator = null, - ) { - } - - /** - * Validates the given DTO and hydrates it if validation is successfull. - * - * Main differences to default validator are: - * - DTO is not hydrated until the first validation group sequence passed successfully. - * - Propertie constraints are validated first - * - Class constraints are validated last after hydration. - * - TypeErrors thrown during hydration are converted to custom constraint violations. - * - * @param object $dto - * @param array|null $groups - */ - public function validateAndHydrate($dto, Request $request, ?array $groups = null): ConstraintViolationListInterface - { - return $this->validateAndHydrateRecursive($dto, $request, $groups); - } - - /** - * @param array|null $groups - */ - private function validateAndHydrateRecursive( - object $dto, - Request $request, - ?array $groups = null, - ?AbstractParam $parent = null, - ?RequestDtoMetadata $dtoMetadata = null - ): ConstraintViolationListInterface { - $className = get_class($dto); - $dtoMetadata ??= $this->metadataFactory->getMetadataFor($className); - - if (empty($groups)) { - $groups = [Constraint::DEFAULT_GROUP, $className]; - } - - $groupSequence = $dtoMetadata->getGroupSequence() ?? $groups; - - $violations = new ConstraintViolationList(); - - /** @var array $hydrationCache */ - $hydrationCache = []; - - $hydrated = false; - - $newGroupSequence = $this->groupSequenceExtractor->getGroupSequence($dto, $groups, $dtoMetadata->getValidatorMetadata()); - - $groupSequence = $newGroupSequence instanceof GroupSequence ? $newGroupSequence->groups : $newGroupSequence; - - foreach ($groupSequence as $groupToProcess) { - $groupViolations = new ConstraintViolationList(); - - foreach ($dtoMetadata->getPropertyMetadataGenerator() as $propertyMetadata) { - $reflectionProperty = $propertyMetadata->getReflectionProperty(); - $propertyName = $reflectionProperty->getName(); - $attr = $dtoMetadata->getAbstractParamAttributeFromProperty($reflectionProperty, $parent); - - $dtoType = $propertyMetadata->getNestedDtoClassName(); - /** @var ConstraintViolationList|null $propertyViolations */ - $propertyViolations = null; - - if ($attr instanceof QueryParam) { - $builtInType = strtolower($propertyMetadata->getBuiltinType()); - if (!$attr->hasTransformType() && QueryParam::isTransformType($builtInType)) { - $attr->setTransformType($builtInType); - } - } - - $value = $attr->getValueFromRequest($request) ?? $propertyMetadata->getDefaultValue(); - - // When the property is a nested RequestDTO we can recurse into custom validation and skip regular validator validation, because: - // class can either be valid or invalid, of a nested dto is not valid it therefore can also not be assigned to the property. - // therefore no other validation is required. - if (null !== $dtoType) { - if (null !== $value) { - $isArray = $propertyMetadata->isNestedDtoArray(); - - $nestedClassName = $dtoType; - /** @var class-string $nestedClassName */ - $nestedMetadata = $this->metadataFactory->getMetadataFor($nestedClassName); - - $valueArray = $isArray ? $value : [$value]; - - $propertyViolations = new ConstraintViolationList(); - $resultArray = []; - foreach ($valueArray as $i => $nestedValue) { - $instance = $nestedMetadata->newInstance($request); - - $nestedAttr = clone $attr; - if ($isArray && $nestedAttr instanceof AbstractNestedParam) { - $nestedAttr->setIndex($i); - } - - $nestedViolations = $this->validateAndHydrateRecursive( - $instance, - $request, - is_array($groupToProcess) ? $groupToProcess : [$groupToProcess], - $nestedAttr, - $nestedMetadata, - ); - - // Reset invalid object - if ($nestedViolations->count() > 0) { - $propertyViolations = $this->prefixViolations( - $propertyViolations, - // Append index to prefix only when in array mode - $isArray ? ($propertyName."[$i]") : $propertyName, - ); - $instance = null; - } - - $resultArray[] = $instance; - $propertyViolations->addAll($nestedViolations); - } - - // Reset invalid object - if ($propertyViolations->count() > 0) { - $value = null; - } else { - $value = $isArray ? $resultArray : $resultArray[0]; - } - } - } else { - // only validate constrained properties - if ($dtoMetadata->isConstrainedProperty($reflectionProperty)) { - $propertyViolations = $this->validator->validatePropertyValue( - $className, - $propertyName, - $value, - $groupToProcess, - ); - $propertyViolations = $this->prefixViolations($propertyViolations, $propertyName); - } - } - - if (null !== $propertyViolations && $propertyViolations->count() > 0) { - $groupViolations->addAll( - $propertyViolations, - ); - } else { - // Store callable to apply later when validation completed successfully - $hydrationCache[] = [ - 'value' => $value, - 'name' => $propertyName, - 'metadata' => $propertyMetadata, - 'reflectionProperty' => $reflectionProperty, - ]; - } - } - - // 2. Short-circuit: If any property failed in this group, stop everything - if ($groupViolations->count() > 0) { - return $groupViolations; - } - - // After the first sequence passed validation successfully we assume data is valid and hydrate the object - if (false === $hydrated) { - $hydrated = true; - $hydrationViolations = new ConstraintViolationList(); - foreach ($hydrationCache as $cacheItem) { - try { - $cacheItem['reflectionProperty']->setValue($dto, $cacheItem['value']); - } catch (\TypeError $e) { - if (TypeErrorInfo::ERROR_TYPE_PROPERTY !== TypeErrorInfo::getErrorTypeFromTypeError($e)) { - throw $e; - } - $hydrationViolations->add( - $this->createTypeErrorViolation($e, $dto, $cacheItem['metadata'], $cacheItem['value']), - ); - } - } - - if ($hydrationViolations->count() > 0) { - return $hydrationViolations; - } - } - - $classViolations = $this->validator - ->validate($dto, $dtoMetadata->getClassConstraints(), $groupToProcess); - - if ($classViolations->count() > 0) { - return $classViolations; - } - } - - return $violations; - } - - /** - * Creates custom constraint violation for type errors occuring when assigning values to types properties and the type is not matching. - * - * @param \TypeError $error The type error that was thrown - * @param object $object The object being validated - * @param RequestDtoParamMetadata $propertyMetadata The metadata of the property being validated - */ - private function createTypeErrorViolation( - \TypeError $error, - object $object, - RequestDtoParamMetadata $propertyMetadata, - mixed $invalidValue = null - ): ConstraintViolation { - $errorInfo = new TypeErrorInfo($error); - - $template = 'This value should be of type {{ expected }}, {{ given }} given.'; - - $params = [ - '{{ expected }}' => $errorInfo->expectedType, - '{{ given }}' => $errorInfo->actualType, - ]; - - $message = $this->translator?->trans($template, $params, 'validators') ?? $template; - - return new ConstraintViolation( - $message, - $template, - $params, - $object, - $propertyMetadata->getPropertyName(), - $invalidValue, - ); - } - - /** - * Prefixes property paths of constraint violations with parent property name. - */ - private function prefixViolations(ConstraintViolationListInterface $violations, string $prefix): ConstraintViolationList - { - $prefixedList = new ConstraintViolationList(); - foreach ($violations as $violation) { - $path = $prefix; - $violationPath = $violation->getPropertyPath(); - - if (!empty($violationPath) && !str_starts_with($violationPath, '[')) { - $violationPath = '.'.$violationPath; - } - - $path .= $violationPath; - - $prefixedList->add(new ConstraintViolation( - $violation->getMessage(), - $violation->getMessageTemplate(), - $violation->getParameters(), - $violation->getRoot(), - $path, - $violation->getInvalidValue(), - $violation->getPlural(), - $violation->getCode(), - $violation->getConstraint(), - $violation->getCause(), - )); - } - - return $prefixedList; - } -} diff --git a/test/Fixtures/CircularReferencingDto.php b/test/Fixtures/CircularReferencingDto.php new file mode 100644 index 0000000..8cb6642 --- /dev/null +++ b/test/Fixtures/CircularReferencingDto.php @@ -0,0 +1,30 @@ +getValue('first')) { + if ('validate_second' === $object->first) { $groups[] = 'First'; } diff --git a/test/Fixtures/GroupSequenceProviderDTO.php b/test/Fixtures/GroupSequenceProviderDTO.php index 7d8203f..d9cbd10 100644 --- a/test/Fixtures/GroupSequenceProviderDTO.php +++ b/test/Fixtures/GroupSequenceProviderDTO.php @@ -15,7 +15,6 @@ use Crtl\RequestDtoResolverBundle\Attribute\BodyParam; use Crtl\RequestDtoResolverBundle\Attribute\RequestDto; -use Crtl\RequestDtoResolverBundle\Trait\RequestDtoTrait; use Symfony\Component\Validator\Constraints as Assert; use Symfony\Component\Validator\GroupSequenceProviderInterface; @@ -23,8 +22,6 @@ #[Assert\GroupSequenceProvider] final class GroupSequenceProviderDTO implements GroupSequenceProviderInterface { - use RequestDtoTrait; - #[BodyParam] public ?string $first = null; @@ -39,7 +36,7 @@ public function getGroupSequence(): array { $groups = [self::class]; - if ('validate_second' === $this->getValue('first')) { + if ('validate_second' === $this->first) { $groups[] = 'First'; } diff --git a/test/Fixtures/StrictTypesDto.php b/test/Fixtures/StrictTypesDto.php index 07227ae..2d6bf84 100644 --- a/test/Fixtures/StrictTypesDto.php +++ b/test/Fixtures/StrictTypesDto.php @@ -28,14 +28,14 @@ final class StrictTypesDto public ?string $nullableString; #[BodyParam] - #[Assert\NotBlank] + #[Assert\GreaterThan(0)] public int $int; #[BodyParam] public ?int $nullableInt; #[BodyParam] - #[Assert\NotBlank] + #[Assert\GreaterThan(0.0)] public float $float; #[BodyParam] diff --git a/test/Integration/RequestDtoFactoryIntegrationTest.php b/test/Integration/RequestDtoFactoryIntegrationTest.php new file mode 100644 index 0000000..8954b41 --- /dev/null +++ b/test/Integration/RequestDtoFactoryIntegrationTest.php @@ -0,0 +1,182 @@ +get(RequestDtoFactory::class); + $this->factory = $factory; + } + + public function testFromRequestThrowsReflectionExceptionWhenClassDoesNotExist(): void + { + $this->expectException(\ReflectionException::class); + // @phpstan-ignore argument.type + $this->factory->fromRequest('NonExistingClass', new Request()); + } + + public function testFromRequestHydratesAllParamTypes(): void + { + $request = new Request( + query: ['query' => 'query_val'], + request: ['body' => 'body_val'], + attributes: ['_route_params' => ['_route' => 'test_route']], + server: ['HTTP_USER_AGENT' => 'test_ua'], + ); + + /** @var AllParamTypesDTO $dto */ + $dto = $this->factory->fromRequest(AllParamTypesDTO::class, $request); + + // @phpstan-ignore method.alreadyNarrowedType + $this->assertInstanceOf(AllParamTypesDTO::class, $dto); + $this->assertEquals('query_val', $dto->query); + $this->assertEquals('body_val', $dto->body); + $this->assertEquals('test_route', $dto->routeName); + $this->assertEquals('test_ua', $dto->userAgent); + } + + public function testFromRequestHydratesNestedDtos(): void + { + $request = new Request( + request: [ + 'children' => [ + ['childName' => 'child1'], + ['childName' => 'child2'], + ] + ], + ); + + /** @var DtoWithNestedDtoArray $dto */ + $dto = $this->factory->fromRequest(DtoWithNestedDtoArray::class, $request); + + // @phpstan-ignore method.alreadyNarrowedType + $this->assertInstanceOf(DtoWithNestedDtoArray::class, $dto); + $this->assertCount(2, $dto->children); + + // @phpstan-ignore method.alreadyNarrowedType + $this->assertInstanceOf(NestedChildDTO::class, $dto->children[0]); + $this->assertEquals('child1', $dto->children[0]->childName); + + // @phpstan-ignore method.alreadyNarrowedType + $this->assertInstanceOf(NestedChildDTO::class, $dto->children[1]); + $this->assertEquals('child2', $dto->children[1]->childName); + } + + public function testFromArrayHydratesDto(): void + { + $data = [ + 'query' => 'q', + 'body' => 'b', + 'routeName' => 'r', + 'userAgent' => 'ua' + ]; + + /** @var AllParamTypesDTO $dto */ + $dto = $this->factory->fromArray(AllParamTypesDTO::class, $data); + + // @phpstan-ignore method.alreadyNarrowedType + $this->assertInstanceOf(AllParamTypesDTO::class, $dto); + $this->assertEquals('q', $dto->query); + $this->assertEquals('b', $dto->body); + $this->assertEquals('r', $dto->routeName); + $this->assertEquals('ua', $dto->userAgent); + } + + public function testFromArrayThrowsExceptionOnTypeError(): void + { + $data = [ + 'childName' => ['invalid'], // Expected string, got array + ]; + + $this->expectException(RequestDtoHydrationException::class); + + try { + $this->factory->fromArray(NestedChildDTO::class, $data); + } catch (RequestDtoHydrationException $e) { + $this->assertCount(1, $e->violations); + // The violation path is prefixed with the property name in fromArrayRecursive + $this->assertEquals('childName', $e->violations[0]->getPropertyPath()); + throw $e; + } + } + + public function testFromArrayThrowsExceptionWithNestedArrayRequestDtoTypeMismatch(): void + { + self::expectException(RequestDtoHydrationException::class); + try { + $this->factory->fromArray( + DtoWithNestedDtoArray::class, + ['children' => [ + [], // No name at all an property is not nullable + ['childName' => 1], // Int despite string is expected + ['childName' => 1.0], // float despite string is expected + ['childName' => false], // bool despite string is expected + ['childName' => new \stdClass()], // object despite string is expected + ]], + ); + } catch (RequestDtoHydrationException $e) { + $this->assertCount(5, $e->violations); + self::assertSame('children[0].childName', $e->violations[0]->getPropertyPath()); + self::assertSame('children[1].childName', $e->violations[1]->getPropertyPath()); + self::assertSame('children[2].childName', $e->violations[2]->getPropertyPath()); + self::assertSame('children[3].childName', $e->violations[3]->getPropertyPath()); + self::assertSame('children[4].childName', $e->violations[4]->getPropertyPath()); + + throw $e; + } + } + + public function testFromArrayThrowsCircularReferenceExceptionWhenCircularReferenceIsDetected(): void + { + self::expectException(CircularReferenceException::class); + $this->factory->fromArray(CircularReferencingDto::class, ['prop' => []]); + } + + public function testFromArrayThrowsCircularReferenceExceptionWhenNestedCircularReferenceIsDetected(): void + { + self::expectException(CircularReferenceException::class); + $this->factory->fromArray(CircularReferencingDto::class, ['array' => [[]]]); + } + + public function testFromRequestThrowsCircularReferenceExceptionWhenCircularReferenceIsDetected(): void + { + self::expectException(CircularReferenceException::class); + $request = new Request(request: ['prop' => []]); + $this->factory->fromRequest(CircularReferencingDto::class, $request); + } + + public function testFromRequestThrowsCircularReferenceExceptionWhenNestedCircularReferenceIsDetected(): void + { + self::expectException(CircularReferenceException::class); + $request = new Request(request: ['array' => [[]]]); + $this->factory->fromRequest(CircularReferencingDto::class, $request); + } +} diff --git a/test/Integration/RequestDtoResolverBundleIntegrationTest.php b/test/Integration/RequestDtoResolverBundleIntegrationTest.php index 6013876..1a0aa18 100644 --- a/test/Integration/RequestDtoResolverBundleIntegrationTest.php +++ b/test/Integration/RequestDtoResolverBundleIntegrationTest.php @@ -93,17 +93,51 @@ public function testHydratesAndValidatesARequestDtoAndCallsTheController(): void self::assertNull($data['nullableArray']); } + public function testReturns400WhenHydrationFails(): void + { + self::bootKernel(); + $kernel = self::$kernel; + + // Sending null for non-nullable typed properties causes hydration TypeErrors + $payload = [ + 'string' => null, // TypeError: cannot assign null to string + 'int' => null, // TypeError: cannot assign null to int + 'float' => null, // TypeError: cannot assign null to float + 'bool' => null, // TypeError: cannot assign null to bool + 'array' => null, // TypeError: cannot assign null to array + ]; + + $request = Request::create( + uri: '/_test', + method: 'POST', + server: [ + 'CONTENT_TYPE' => 'application/json', + 'HTTP_ACCEPT' => 'application/json', + ], + content: json_encode($payload, JSON_THROW_ON_ERROR), + ); + + $controller = new StrictTypesDtoController(); + + $request->attributes->set('_controller', $controller); + + $response = $kernel->handle($request); + + self::assertValidationErrorResponse($response, ['string', 'int', 'float', 'bool', 'array']); + } + public function testReturns400WhenValidationFails(): void { self::bootKernel(); $kernel = self::$kernel; + // Values with correct types but failing validation constraints $payload = [ 'string' => '', // NotBlank violation - 'int' => null, // NotBlank considers 0 as blank -> violation (Symfony behavior) - 'float' => null, // NotBlank considers 0.0 as blank -> violation - 'bool' => false, // NotBlank considers false as blank -> violation - 'array' => [], // NotBlank considers empty array as blank -> violation + 'int' => 0, // NotBlank considers 0 as blank + 'float' => 0.0, // NotBlank considers 0.0 as blank + 'bool' => false, // NotBlank considers false as blank + 'array' => [], // NotBlank considers empty array as blank ]; $request = Request::create( @@ -120,11 +154,7 @@ public function testReturns400WhenValidationFails(): void $request->attributes->set('_controller', $controller); - // Replace with your concrete exception type: - // e.g. \Crtl\RequestDtoResolverBundle\Exception\RequestDtoValidationException::class - $response = $kernel->handle($request); - var_dump($response->getContent()); self::assertValidationErrorResponse($response, ['string', 'int', 'float', 'bool', 'array']); } @@ -222,7 +252,6 @@ public function __invoke(ExampleDto $dto): JsonResponse $request->attributes->set('_controller', $controller); $response = $kernel->handle($request); - echo $response->getContent(); self::assertSame(400, $response->getStatusCode()); } @@ -303,7 +332,6 @@ public function __invoke(GroupSequenceProviderDTO $dto): JsonResponse $request->attributes->set('_controller', $controller); $response = $kernel->handle($request); - echo $response->getContent(); self::assertSame($expectedStatus, $response->getStatusCode()); @@ -408,7 +436,6 @@ public function __invoke(CollectionPathTestDto $dto): JsonResponse $request->attributes->set('_controller', $controller); $response = $kernel->handle($request); - var_dump($response->getContent()); self::assertValidationErrorResponse($response, [ 'property[0][key]', diff --git a/test/Integration/RequestDtoValidatorIntegrationTest.php b/test/Integration/RequestDtoValidatorIntegrationTest.php deleted file mode 100644 index 904365d..0000000 --- a/test/Integration/RequestDtoValidatorIntegrationTest.php +++ /dev/null @@ -1,300 +0,0 @@ -password) && isset($object->passwordConfirm) && $object->password !== $object->passwordConfirm) { - $context->buildViolation('Passwords do not match') - ->addViolation(); - } - } -} - -#[RequestDto] -final class NestedParentDto -{ - #[QueryParam('title')] - #[Assert\NotBlank] - public string $title = ''; - - #[QueryParam('child')] - #[Assert\Valid] - public BasicTestDto $child; -} - -#[RequestDto] -#[Assert\GroupSequence(['Basic', 'Strict', 'GroupSequenceDto'])] -final class GroupSequenceDto -{ - #[QueryParam('name')] - #[Assert\NotBlank(groups: ['Basic'])] - public string $name; - - #[QueryParam('email')] - #[Assert\Email(groups: ['Strict'])] - public string $email; -} - -#[RequestDto] -final class BodyParamDto -{ - #[BodyParam('name')] - #[Assert\NotBlank] - public string $name; - - #[BodyParam('email')] - #[Assert\Email] - public string $email; -} - -#[RequestDto] -final class MixedParamDto -{ - #[QueryParam('q')] - #[Assert\NotBlank] - public string $query; - - #[BodyParam('b')] - #[Assert\NotBlank] - public string $body; -} - -#[RequestDto] -final class GroupsDto -{ - #[QueryParam('name')] - #[Assert\NotBlank(groups: ['Registration'])] - public string $name; - - #[QueryParam('age')] - #[Assert\GreaterThan(18, groups: ['Adult'])] - public int $age; -} - -final class RequestDtoValidatorIntegrationTest extends TestCase -{ - private RequestDtoValidator $validator; - - protected function setUp(): void - { - $innerValidator = Validation::createValidatorBuilder() - ->enableAttributeMapping() - ->getValidator(); - - $extractorFactory = new PropertyInfoExtractorFactory( - new PhpDocExtractor(), - new ReflectionExtractor(), - ); - - $propertyInfoExtractor = $extractorFactory->create(); - - $reflectionHelper = new DtoReflectionHelper(); - $paramMetadataFactory = new RequestDtoParamMetadataFactory($innerValidator, $reflectionHelper, $propertyInfoExtractor); - $metadataFactory = new RequestDtoMetadataFactory($innerValidator, $reflectionHelper, $paramMetadataFactory); - $groupSequenceExtractor = new GroupSequenceExtractor(); - $this->validator = new RequestDtoValidator($innerValidator, $metadataFactory, $groupSequenceExtractor); - } - - // @phpstan-ignore method.unused - private function debugDumpViolations(ConstraintViolationListInterface $violations): void - { - foreach ($violations as $violation) { - dump($violation->getPropertyPath().': '.$violation->getMessage()); - } - } - - public function testValidateAndHydrateSuccess(): void - { - $dto = new BasicTestDto(); - $request = new Request(['name' => 'John Doe', 'age' => 25]); - - $violations = $this->validator->validateAndHydrate($dto, $request, []); - - $this->assertCount(0, $violations); - $this->assertEquals('John Doe', $dto->name); - $this->assertEquals(25, $dto->age); - } - - public function testValidateAndHydratePropertyFailure(): void - { - $dto = new BasicTestDto(); - // age is too young, name is too short - $request = new Request(['name' => 'Jo', 'age' => 15]); - - $violations = $this->validator->validateAndHydrate($dto, $request, []); - - $this->assertGreaterThan(0, $violations->count()); - // Properties should NOT be set because validation failed - $this->assertFalse(isset($dto->name)); - $this->assertFalse(isset($dto->age)); - } - - public function testClassConstraintFailure(): void - { - $dto = new ClassConstraintDto(); - $request = new Request(['password' => 'foo', 'passwordConfirm' => 'bar']); - - $violations = $this->validator->validateAndHydrate($dto, $request, []); - - $this->assertCount(1, $violations); - $this->assertEquals('Passwords do not match', $violations[0]->getMessage()); - // Properties SHOULD be set because property-level validation passed - $this->assertEquals('foo', $dto->password); - $this->assertEquals('bar', $dto->passwordConfirm); - } - - public function testNestedDtoValidation(): void - { - $dto = new NestedParentDto(); - $request = new Request([ - 'title' => 'Parent Title', - 'child' => [ - 'name' => 'Child Name', - 'age' => 20, - ], - ]); - - $violations = $this->validator->validateAndHydrate($dto, $request, []); - $this->assertCount(0, $violations); - $this->assertEquals('Parent Title', $dto->title); - $this->assertEquals('Child Name', $dto->child->name); - $this->assertEquals(20, $dto->child->age); - } - - public function testNestedDtoFailure(): void - { - $dto = new NestedParentDto(); - $request = new Request([ - 'title' => 'Parent Title', - 'child' => [ - 'name' => 'Jo', // too short - 'age' => 20, - ], - ]); - - $violations = $this->validator->validateAndHydrate($dto, $request, []); - - $this->assertGreaterThan(0, $violations->count()); - // child should NOT be set because its internal validation failed - $this->assertFalse(isset($dto->child)); - } - - public function testGroupSequenceShortCircuit(): void - { - $dto = new GroupSequenceDto(); - // name is empty (fails Basic group), email is invalid (fails Strict group) - $request = new Request(['name' => '', 'email' => 'invalid-email']); - - $violations = $this->validator->validateAndHydrate($dto, $request, []); - - // It should only report violation for 'name' because it short-circuits after the first group in sequence fails - $this->assertCount(1, $violations); - // Note: currently property path is empty due to how validatePropertyValue is called in RequestDtoValidator - // $this->assertEquals('name', $violations[0]->getPropertyPath()); - } - - public function testBodyParamValidation(): void - { - $dto = new BodyParamDto(); - $request = new Request([], ['name' => 'John', 'email' => 'john@example.com']); - - $violations = $this->validator->validateAndHydrate($dto, $request, []); - - $this->assertCount(0, $violations); - $this->assertEquals('John', $dto->name); - $this->assertEquals('john@example.com', $dto->email); - } - - public function testMixedParamsValidation(): void - { - $dto = new MixedParamDto(); - $request = new Request(['q' => 'search'], ['b' => 'content']); - - $violations = $this->validator->validateAndHydrate($dto, $request, []); - - $this->assertCount(0, $violations); - $this->assertEquals('search', $dto->query); - $this->assertEquals('content', $dto->body); - } - - public function testValidationGroups(): void - { - $dto = new GroupsDto(); - $request = new Request(['name' => '', 'age' => 15]); - - // Validate only Registration group -> name should fail, age should be ignored - $violations = $this->validator->validateAndHydrate($dto, $request, ['Registration']); - $this->assertCount(1, $violations); - - // Validate only Adult group -> age should fail, name should be ignored - $dto = new GroupsDto(); - $violations = $this->validator->validateAndHydrate($dto, $request, ['Adult']); - $this->assertCount(1, $violations); - - // Validate both -> both should fail - $dto = new GroupsDto(); - $violations = $this->validator->validateAndHydrate($dto, $request, ['Registration', 'Adult']); - // Note: RequestDtoValidator processes groups one by one and stops if a group has violations. - // If 'Registration' fails, it might stop there if it's considered a sequence or if that's how it's implemented. - // Actually looking at code: it iterates groups and returns if $groupViolations > 0. - $this->assertCount(1, $violations); - } -} diff --git a/test/Unit/EventSubscriber/RequestDtoValidationEventSubscriberTest.php b/test/Unit/EventSubscriber/RequestDtoValidationEventSubscriberTest.php index e923444..10cc0e9 100644 --- a/test/Unit/EventSubscriber/RequestDtoValidationEventSubscriberTest.php +++ b/test/Unit/EventSubscriber/RequestDtoValidationEventSubscriberTest.php @@ -16,7 +16,6 @@ use Crtl\RequestDtoResolverBundle\EventSubscriber\RequestDtoValidationEventSubscriber; use Crtl\RequestDtoResolverBundle\Exception\RequestValidationException; use Crtl\RequestDtoResolverBundle\Utility\DtoInstanceBagInterface; -use Crtl\RequestDtoResolverBundle\Validator\RequestDtoValidator; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; @@ -24,18 +23,20 @@ use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\HttpKernel\KernelInterface; +use Symfony\Component\Validator\ConstraintViolationList; use Symfony\Component\Validator\ConstraintViolationListInterface; +use Symfony\Component\Validator\Validator\ValidatorInterface; final class RequestDtoValidationEventSubscriberTest extends TestCase { private RequestDtoValidationEventSubscriber $subscriber; - private RequestDtoValidator&MockObject $validatorMock; + private ValidatorInterface&MockObject $validatorMock; private DtoInstanceBagInterface&MockObject $bag; protected function setUp(): void { - $this->validatorMock = $this->createMock(RequestDtoValidator::class); + $this->validatorMock = $this->createMock(ValidatorInterface::class); $this->bag = $this->createMock(DtoInstanceBagInterface::class); $this->subscriber = new RequestDtoValidationEventSubscriber( $this->validatorMock, @@ -73,10 +74,12 @@ public function testOnKernelControllerArgumentsThrowsRequestValidationExceptionW \stdClass::class => $testDto, ]); + $this->bag->method('getHydrationViolations')->willReturn(null); + $violations = $this->createMock(ConstraintViolationListInterface::class); $violations->method('count')->willReturn(1); - $this->validatorMock->method('validateAndHydrate')->with($testDto)->willReturn($violations); + $this->validatorMock->method('validate')->with($testDto)->willReturn($violations); $event = $this->createTestEvent(HttpKernelInterface::MAIN_REQUEST, $request); @@ -84,7 +87,7 @@ public function testOnKernelControllerArgumentsThrowsRequestValidationExceptionW $this->subscriber->onKernelControllerArguments($event); } - public function testOnKernelControllerArgumentsDoesNothingWhenDTOIsValid(): void + public function testOnKernelControllerArgumentsMergesHydrationViolationsWithValidatorViolations(): void { $testDto = new \stdClass(); @@ -94,15 +97,49 @@ public function testOnKernelControllerArgumentsDoesNothingWhenDTOIsValid(): void ]); $violations = $this->createMock(ConstraintViolationListInterface::class); - $violations + $violations->method('count')->willReturn(1); + + $this->bag->method('getHydrationViolations') + ->with(\stdClass::class, $request) + ->willReturn($violations); + + $validatorViolations = $this->createMock(ConstraintViolationListInterface::class); + $this->validatorMock + ->expects(self::once()) + ->method('validate') + ->with($testDto) + ->willReturn($validatorViolations) + ; + + $validatorViolations->method('count')->willReturn(1); + $validatorViolations ->expects(self::once()) - ->method('count') - ->willReturn(0) + ->method('addAll') + ->with($violations) ; + $event = $this->createTestEvent(HttpKernelInterface::MAIN_REQUEST, $request); + + self::expectException(RequestValidationException::class); + $this->subscriber->onKernelControllerArguments($event); + } + + public function testOnKernelControllerArgumentsDoesNothingWhenDTOIsValid(): void + { + $testDto = new \stdClass(); + + $request = new Request(); + $this->bag->method('getRegisteredInstances')->with($request)->willReturn([ + \stdClass::class => $testDto, + ]); + + $this->bag->method('getHydrationViolations')->willReturn(null); + + $violations = new ConstraintViolationList(); + $this->validatorMock ->expects(self::once()) - ->method('validateAndHydrate') + ->method('validate') ->with($testDto) ->willReturn($violations) ; diff --git a/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php b/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php index 734fd3c..dd63388 100644 --- a/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php +++ b/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php @@ -172,7 +172,7 @@ public function testGetMetadataForThrowsRuntimeExceptionWhenUnsupportedTypeIsEnc } } -class DummyDto +final class DummyDto { public string $prop1; diff --git a/test/Unit/Reflection/RequestDtoParamMetadataTest.php b/test/Unit/Reflection/RequestDtoParamMetadataTest.php index dec16e3..87badd7 100644 --- a/test/Unit/Reflection/RequestDtoParamMetadataTest.php +++ b/test/Unit/Reflection/RequestDtoParamMetadataTest.php @@ -158,7 +158,7 @@ public function testSerializeAndUnserializeWorksCorrectly(): void } } -class ParamMetadataDummyDto +final class ParamMetadataDummyDto { #[QueryParam] public string $prop; @@ -170,6 +170,6 @@ class ParamMetadataDummyDto public string $multipleAttributesProp; } -class ParamMetadataNestedDto +final class ParamMetadataNestedDto { } diff --git a/test/Unit/RequestDtoResolverTest.php b/test/Unit/RequestDtoResolverTest.php index 775bbe8..b149b95 100644 --- a/test/Unit/RequestDtoResolverTest.php +++ b/test/Unit/RequestDtoResolverTest.php @@ -12,28 +12,13 @@ declare(strict_types=1); /** @noinspection PhpClassCantBeUsedAsAttributeInspection */ -/** @noinspection PhpClassCantBeUsedAsAttributeInspection */ -/** @noinspection PhpClassCantBeUsedAsAttributeInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ -/** @noinspection PhpUnhandledExceptionInspection */ - /** @noinspection PhpUnhandledExceptionInspection */ namespace Crtl\RequestDtoResolverBundle\Test\Unit; use Crtl\RequestDtoResolverBundle\Attribute; -use Crtl\RequestDtoResolverBundle\Reflection\RequestDtoMetadata; -use Crtl\RequestDtoResolverBundle\Reflection\RequestDtoMetadataFactory; +use Crtl\RequestDtoResolverBundle\Factory\Exception\RequestDtoHydrationException; +use Crtl\RequestDtoResolverBundle\Factory\RequestDtoFactory; use Crtl\RequestDtoResolverBundle\RequestDtoResolver; use Crtl\RequestDtoResolverBundle\Utility\DtoInstanceBagInterface; use Crtl\RequestDtoResolverBundle\Utility\DtoReflectionHelper; @@ -42,6 +27,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; +use Symfony\Component\Validator\ConstraintViolationList; #[Attribute\RequestDto] final class UnitRequestDTO @@ -55,7 +41,7 @@ final class RequestDtoResolverTest extends TestCase private DtoInstanceBagInterface&MockObject $bag; private DtoReflectionHelper&MockObject $reflectionHelper; - private RequestDtoMetadataFactory&MockObject $metadataFactory; + private RequestDtoFactory&MockObject $factory; /** * @throws Exception @@ -64,8 +50,8 @@ protected function setUp(): void { $this->bag = $this->createMock(DtoInstanceBagInterface::class); $this->reflectionHelper = $this->createMock(DtoReflectionHelper::class); - $this->metadataFactory = $this->createMock(RequestDtoMetadataFactory::class); - $this->resolver = new RequestDtoResolver($this->bag, $this->metadataFactory, $this->reflectionHelper); + $this->factory = $this->createMock(RequestDtoFactory::class); + $this->resolver = new RequestDtoResolver($this->bag, $this->reflectionHelper, $this->factory); } public function testResolveReturnsEmptyArrayIfArgumentTypeIsNotRequestDto(): void @@ -84,28 +70,34 @@ public function testResolveReturnsEmptyArrayIfArgumentTypeIsNotRequestDto(): voi $this->assertEmpty($result); } - public function testResolveThrowsRuntimeExceptionIfDTOInstantiationFails(): void + public function testResolveReturnsArrayWithDTOAndRegistersItInDtoInstanceBag(): void { - $metadataMock = $this->createMock(RequestDtoMetadata::class); - $metadataMock->expects(self::once()) - ->method('newInstance')->willReturn(null); - - $this->metadataFactory->expects(self::once()) - ->method('getMetadataFor')->willReturn($metadataMock); + $request = new Request(); + $argument = new ArgumentMetadata('test', UnitRequestDTO::class, false, false, null); $this->reflectionHelper->expects($this->once()) ->method('isRequestDto') ->with(UnitRequestDTO::class) ->willReturn(true); - $argument = new ArgumentMetadata('test', UnitRequestDTO::class, false, false, null); + $dto = new UnitRequestDTO(); + $this->factory->expects($this->once()) + ->method('fromRequest') + ->with(UnitRequestDTO::class, $request) + ->willReturn($dto); - self::expectException(\RuntimeException::class); - self::expectExceptionMessage('Failed to instantiate request dto '.UnitRequestDTO::class); - $this->resolver->resolve(new Request(), $argument); + $this->bag->expects($this->once()) + ->method('registerInstance') + ->with($this->isInstanceOf(UnitRequestDTO::class), $request); + + $result = $this->resolver->resolve($request, $argument); + + $this->assertIsArray($result); + $this->assertCount(1, $result); + $this->assertInstanceOf(UnitRequestDTO::class, $result[0]); } - public function testResolveReturnsArrayWithDTOAndRegistersItInDtoInstanceBag(): void + public function testResolveStoresHydrationViolationsWhenFactoryThrows(): void { $request = new Request(); $argument = new ArgumentMetadata('test', UnitRequestDTO::class, false, false, null); @@ -115,25 +107,25 @@ public function testResolveReturnsArrayWithDTOAndRegistersItInDtoInstanceBag(): ->with(UnitRequestDTO::class) ->willReturn(true); - $metadata = $this->createMock(RequestDtoMetadata::class); - $metadata->expects($this->once()) - ->method('newInstance') - ->with($request) - ->willReturn(new UnitRequestDTO()); + $dto = new UnitRequestDTO(); + $violations = new ConstraintViolationList(); - $this->metadataFactory->expects($this->once()) - ->method('getMetadataFor') - ->with(UnitRequestDTO::class) - ->willReturn($metadata); + $this->factory->expects($this->once()) + ->method('fromRequest') + ->willThrowException(new RequestDtoHydrationException($dto, $violations)); + + $this->bag->expects($this->once()) + ->method('registerHydrationViolations') + ->with(UnitRequestDTO::class, $violations, $request); $this->bag->expects($this->once()) ->method('registerInstance') - ->with($this->isInstanceOf(UnitRequestDTO::class), $request); + ->with($dto, $request); $result = $this->resolver->resolve($request, $argument); $this->assertIsArray($result); $this->assertCount(1, $result); - $this->assertInstanceOf(UnitRequestDTO::class, $result[0]); + $this->assertSame($dto, $result[0]); } } diff --git a/test/Unit/Trait/RequestDtoTraitTest.php b/test/Unit/Trait/RequestDtoTraitTest.php deleted file mode 100644 index d43482d..0000000 --- a/test/Unit/Trait/RequestDtoTraitTest.php +++ /dev/null @@ -1,128 +0,0 @@ -request = $request; - } -} -final class RequestDtoTraitTest extends TestCase -{ - public function testGetValueReturnsRequestBodyValueWhenNoAttributeIsPresent(): void - { - $request = new Request(request: ['noAttr' => 'body-value']); - $dto = new RequestDtoTraitTestDto($request); - - $this->assertEquals('body-value', $dto->getValue('noAttr')); - } - - public function testGetValueReturnsQueryParamValue(): void - { - $request = new Request(query: ['queryAttr' => 'query-value']); - $dto = new RequestDtoTraitTestDto($request); - - $this->assertEquals('query-value', $dto->getValue('queryAttr')); - } - - public function testGetValueReturnsBodyParamValue(): void - { - $request = new Request(request: ['bodyAttr' => 'body-value']); - $dto = new RequestDtoTraitTestDto($request); - - $this->assertEquals('body-value', $dto->getValue('bodyAttr')); - } - - public function testGetValueReturnsHeaderParamValue(): void - { - $request = new Request(); - $request->headers->set('X-Test-Header', 'header-value'); - $dto = new RequestDtoTraitTestDto($request); - - $this->assertEquals('header-value', $dto->getValue('headerAttr')); - } - - public function testGetValueReturnsRouteParamValue(): void - { - $request = new Request(attributes: ['_route_params' => ['id' => 'route-value']]); - $dto = new RequestDtoTraitTestDto($request); - - $this->assertEquals('route-value', $dto->getValue('routeAttr')); - } - - public function testGetValueReturnsFileParamValue(): void - { - $file = $this->createMock(UploadedFile::class); - $request = new Request(files: ['fileAttr' => $file]); - $dto = new RequestDtoTraitTestDto($request); - - $this->assertSame($file, $dto->getValue('fileAttr')); - } - - public function testGetValueThrowsLogicExceptionWhenRequestIsNull(): void - { - $dto = new RequestDtoTraitTestDto(null); - - $this->expectException(\LogicException::class); - $this->expectExceptionMessage('Request must be set before calling getValue().'); - - $dto->getValue('noAttr'); - } - - public function testGetValueThrowsReflectionExceptionWhenPropertyDoesNotExist(): void - { - $request = new Request(); - $dto = new RequestDtoTraitTestDto($request); - - $this->expectException(\ReflectionException::class); - - $dto->getValue('nonExistentProperty'); - } - - public function testGetValueReturnsPropertyValueIfInitialized(): void - { - $request = $this->createMock(Request::class); - $dto = new RequestDtoTraitTestDto($request); - $dto->noAttr = 'test'; - - $this->assertEquals('test', $dto->getValue('noAttr')); - } -} diff --git a/test/Unit/Utility/DtoInstanceBagTest.php b/test/Unit/Utility/DtoInstanceBagTest.php index 744145b..f4a0673 100644 --- a/test/Unit/Utility/DtoInstanceBagTest.php +++ b/test/Unit/Utility/DtoInstanceBagTest.php @@ -16,6 +16,7 @@ use Crtl\RequestDtoResolverBundle\Utility\DtoInstanceBag; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Validator\ConstraintViolationList; final class DtoInstanceBagTest extends TestCase { @@ -79,4 +80,24 @@ public function testGetRegisteredInstanceReturnsInstanceOrNullIfNotFound(): void $this->assertSame($instance, $this->bag->getRegisteredInstance(get_class($instance), $this->request)); } + + public function testRegisterAndGetHydrationViolations(): void + { + $violations = new ConstraintViolationList(); + + $this->assertNull($this->bag->getHydrationViolations(\stdClass::class, $this->request)); + + $this->bag->registerHydrationViolations(\stdClass::class, $violations, $this->request); + + $this->assertSame($violations, $this->bag->getHydrationViolations(\stdClass::class, $this->request)); + } + + public function testGetHydrationViolationsReturnsNullForUnregisteredClass(): void + { + $violations = new ConstraintViolationList(); + $this->bag->registerHydrationViolations(\stdClass::class, $violations, $this->request); + + // @phpstan-ignore argument.type + $this->assertNull($this->bag->getHydrationViolations('NonExistentClass', $this->request)); + } } diff --git a/test/Unit/Utility/DtoReflectionHelperTest.php b/test/Unit/Utility/DtoReflectionHelperTest.php index 8683b92..2623c62 100644 --- a/test/Unit/Utility/DtoReflectionHelperTest.php +++ b/test/Unit/Utility/DtoReflectionHelperTest.php @@ -139,10 +139,14 @@ public function testIsPropertyAttributed(): void public function testIsRequestDto(): void { + // @phpstan-ignore method.alreadyNarrowedType $this->assertTrue($this->helper->isRequestDto(AllParamTypesDTO::class)); + // @phpstan-ignore method.alreadyNarrowedType $this->assertTrue($this->helper->isRequestDto(new AllParamTypesDTO())); + // @phpstan-ignore method.alreadyNarrowedType $this->assertTrue($this->helper->isRequestDto(new \ReflectionClass(AllParamTypesDTO::class))); + // @phpstan-ignore method.alreadyNarrowedType $this->assertFalse($this->helper->isRequestDto(\stdClass::class)); } } diff --git a/test/Unit/Validator/GroupSequenceExtractorTest.php b/test/Unit/Validator/GroupSequenceExtractorTest.php deleted file mode 100644 index 127db89..0000000 --- a/test/Unit/Validator/GroupSequenceExtractorTest.php +++ /dev/null @@ -1,175 +0,0 @@ -createMock(ClassMetadataInterface::class); - $metadata->method('getClassName')->willReturn(\stdClass::class); - - $result = $extractor->getGroupSequence($object, $groups, $metadata); - - $this->assertEquals($groups, $result); - } - - public function testGetGroupSequenceReturnsGroupsIfClassNameNotInGroups(): void - { - $extractor = new GroupSequenceExtractor(); - $object = new \stdClass(); - $groups = [Constraint::DEFAULT_GROUP]; - $metadata = $this->createMock(ClassMetadataInterface::class); - $metadata->method('getClassName')->willReturn('SomeOtherClass'); - - $result = $extractor->getGroupSequence($object, $groups, $metadata); - - $this->assertEquals($groups, $result); - } - - public function testGetGroupSequenceReturnsStaticSequenceFromMetadata(): void - { - $extractor = new GroupSequenceExtractor(); - $object = new \stdClass(); - $className = \stdClass::class; - $groups = [$className, Constraint::DEFAULT_GROUP]; - $sequence = new GroupSequence(['Group1', 'Group2']); - - $metadata = $this->createMock(ClassMetadataInterface::class); - $metadata->method('getClassName')->willReturn($className); - $metadata->method('hasGroupSequence')->willReturn(true); - $metadata->method('getGroupSequence')->willReturn($sequence); - - $result = $extractor->getGroupSequence($object, $groups, $metadata); - - $this->assertSame($sequence, $result); - } - - public function testGetGroupSequenceReturnsSequenceFromGroupSequenceProviderInterface(): void - { - $extractor = new GroupSequenceExtractor(); - $object = $this->createMock(GroupSequenceProviderInterface::class); - $className = get_class($object); - $groups = [$className, Constraint::DEFAULT_GROUP]; - $sequence = ['Group1', 'Group2']; - - $object->method('getGroupSequence')->willReturn($sequence); - - $metadata = $this->createMock(ClassMetadataInterface::class); - $metadata->method('getClassName')->willReturn($className); - $metadata->method('hasGroupSequence')->willReturn(false); - $metadata->method('isGroupSequenceProvider')->willReturn(true); - - // @phpstan-ignore function.alreadyNarrowedType - if (method_exists($metadata, 'getGroupProvider')) { - $metadata->method('getGroupProvider')->willReturn(null); - } - - $result = $extractor->getGroupSequence($object, $groups, $metadata); - - $this->assertInstanceOf(GroupSequence::class, $result); - $this->assertEquals($sequence, $result->groups); - } - - public function testGetGroupSequenceReturnsSequenceFromGroupProviderInterfaceViaLocator(): void - { - // @phpstan-ignore function.alreadyNarrowedType - if (!method_exists(ClassMetadataInterface::class, 'getGroupProvider')) { - self::markTestSkipped('External GroupProviderInterface not available'); - } - - $locator = $this->createMock(ContainerInterface::class); - $extractor = new GroupSequenceExtractor($locator); - - $object = new \stdClass(); - $className = \stdClass::class; - $groups = [$className, Constraint::DEFAULT_GROUP]; - $providerClass = 'App\Validator\MyGroupProvider'; - $sequence = ['GroupA', 'GroupB']; - - $metadata = $this->createMock(ClassMetadataInterface::class); - $metadata->method('getClassName')->willReturn($className); - $metadata->method('hasGroupSequence')->willReturn(false); - $metadata->method('isGroupSequenceProvider')->willReturn(true); - $metadata->method('getGroupProvider')->willReturn($providerClass); - - $provider = $this->createMock(GroupProviderInterface::class); - $provider->method('getGroups')->with($object)->willReturn($sequence); - - $locator->method('get')->with($providerClass)->willReturn($provider); - - $result = $extractor->getGroupSequence($object, $groups, $metadata); - - $this->assertInstanceOf(GroupSequence::class, $result); - $this->assertEquals($sequence, $result->groups); - } - - public function testGetGroupSequenceThrowsLogicExceptionWhenLocatorIsMissing(): void - { - // @phpstan-ignore function.alreadyNarrowedType - if (!method_exists(ClassMetadataInterface::class, 'getGroupProvider')) { - self::markTestSkipped('External GroupProviderInterface not available'); - } - - $extractor = new GroupSequenceExtractor(null); - - $object = new \stdClass(); - $className = \stdClass::class; - $groups = [$className, Constraint::DEFAULT_GROUP]; - $providerClass = 'App\Validator\MyGroupProvider'; - - $metadata = $this->createMock(ClassMetadataInterface::class); - $metadata->method('getClassName')->willReturn($className); - $metadata->method('hasGroupSequence')->willReturn(false); - $metadata->method('isGroupSequenceProvider')->willReturn(true); - // @phpstan-ignore function.alreadyNarrowedType - if (method_exists($metadata, 'getGroupProvider')) { - $metadata->method('getGroupProvider')->willReturn($providerClass); - } - - $this->expectException(\LogicException::class); - $this->expectExceptionMessage('A group provider locator is required when using group provider.'); - - $extractor->getGroupSequence($object, $groups, $metadata); - } - - public function testGetGroupSequenceReturnsGroupsIfNoSequenceOrProviderDefined(): void - { - $extractor = new GroupSequenceExtractor(); - $object = new \stdClass(); - $className = \stdClass::class; - $groups = [$className, Constraint::DEFAULT_GROUP]; - - $metadata = $this->createMock(ClassMetadataInterface::class); - $metadata->method('getClassName')->willReturn($className); - $metadata->method('hasGroupSequence')->willReturn(false); - $metadata->method('isGroupSequenceProvider')->willReturn(false); - - $result = $extractor->getGroupSequence($object, $groups, $metadata); - - $this->assertEquals($groups, $result); - } -} From a8f3a90274379697b630349c9d276706a4b37447 Mon Sep 17 00:00:00 2001 From: Marvin Petker Date: Sun, 8 Feb 2026 04:10:34 +0100 Subject: [PATCH 2/6] feat: Only assign properties when provided with data. - Adds AbstractParam::hasValueInRequest method. - Refactors RequestDtoValidator to only assign values that have value in request. --- README.md | 275 ++++++++---------- src/Attribute/AbstractNestedParam.php | 14 + src/Attribute/AbstractParam.php | 5 + src/Attribute/FileParam.php | 5 + src/Attribute/HeaderParam.php | 5 + src/Attribute/RouteParam.php | 11 + src/Factory/RequestDtoFactory.php | 66 ++++- src/Reflection/RequestDtoParamMetadata.php | 5 + .../RequestDtoFactoryIntegrationTest.php | 11 +- ...equestDtoResolverBundleIntegrationTest.php | 8 +- test/Unit/Attribute/AbstractParamTest.php | 5 + 11 files changed, 242 insertions(+), 168 deletions(-) diff --git a/README.md b/README.md index 0ced1bc..cd579b3 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,4 @@ + # crtl/request-dto-resolver-bundle [![codecov](https://codecov.io/gh/crtl/request-dto-resolver-bundle/branch/2.x/graph/badge.svg?token=VLXDJ925T8)](https://codecov.io/gh/crtl/request-dto-resolver-bundle) @@ -7,42 +8,57 @@ [![License](http://poser.pugx.org/crtl/request-dto-resolver-bundle/license)](https://packagist.org/packages/crtl/request-dto-resolver-bundle) [![PHP Version Require](http://poser.pugx.org/crtl/request-dto-resolver-bundle/require/php)](https://packagist.org/packages/crtl/request-dto-resolver-bundle) +A Symfony bundle for predictable, type-safe instantiation and validation of request DTOs. -Symfony bundle for streamlined instantiation and validation of request DTOs. +It removes boilerplate from controllers while staying close to Symfony’s +native validation and argument resolving mechanisms. ## Features -1. **Automatic DTO Handling**:
- Instantly creates and validates Data Transfer Objects (DTOs) from `Request` data, that are type-hinted in controller actions. -2. **Symfony Validator Integration**:
Leverages Symfony's built-in validator to ensure data integrity and compliance with your validation rules. -3. **Nested DTO Support**:
Handles complex request structures by supporting nested DTOs for both query and body parameters, making it easier to manage hierarchical data. -4. **Strict Typing Support**:
DTO properties can now be strictly typed, ensuring better code quality and IDE support. -5. **Flexible Query Transformation**:
Built-in support for transforming query parameters into specific types (int, float, string, bool) or via custom callbacks. +- **Automatic DTO Resolution** + DTOs type-hinted in controller actions are instantiated and validated automatically. + +- **Native Symfony Validator Integration** + Uses Symfony’s `ValidatorInterface` without custom validation layers. + +- **Nested DTO Support** + Supports complex request payloads with nested DTOs for query, body, header, file and route parameters. + +- **Strict Typing Friendly** + DTO properties can be strictly typed for better IDE support and safer refactoring. +- **Flexible Query Parameter Transformation** + Query parameters can be transformed to scalar types or via custom callbacks. ## Installation ```bash composer require crtl/request-dto-resolver-bundle -``` +```` ## Configuration -Register the bundle in your Symfony application. Add the following to your `config/bundles.php` file: +Register the bundle in your Symfony application: ```php +// config/bundles.php return [ - // other bundles - Crtl\RequestDtoResolverBundle\CrtlRequestDTOResolverBundle::class => ["all" => true], + // ... + Crtl\RequestDtoResolverBundle\CrtlRequestDtoResolverBundle::class => ["all" => true], ]; ``` ## Usage -### Step 1: Create a DTO +### Step 1: Define a Request DTO + +Create a DTO class and annotate it with `#[RequestDto]`. +Use parameter attributes to map request data to properties. -Create a class to represent your request data. -Annotate the class with [`#[RequestDto]`](src/Attribute/RequestDto.php) and use the attributes below for properties to map request parameters. +> **The attribute is required to identify which controller arguments should be resolved and validated.** + + +### 1.1 Strictly typed DTO ```php namespace App\DTO; @@ -53,199 +69,153 @@ use Crtl\RequestDtoResolverBundle\Attribute\HeaderParam; use Crtl\RequestDtoResolverBundle\Attribute\QueryParam; use Crtl\RequestDtoResolverBundle\Attribute\RouteParam; use Crtl\RequestDtoResolverBundle\Attribute\RequestDto; +use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Validator\Constraints as Assert; #[RequestDto] class ExampleDTO { - // DTOs can now be strictly typed. - // Important: Validation constraints must be correct to prevent TypeErrors - // since hydration happens after validation. #[BodyParam, Assert\NotBlank, Assert\Type("string")] public string $someParam; - // Matches file in uploaded files #[FileParam, Assert\NotNull] - public mixed $file; - - // Matches Content-Type header in headers + public ?UploadedFile $file; + #[HeaderParam("Content-Type"), Assert\NotBlank] public string $contentType; - - // QueryParam supports optional transformType: "int", "float", "string", "bool" - // or a custom callback: fn(string $val) => ... + #[QueryParam(name: "age", transformType: "int"), Assert\GreaterThan(18)] public int $age; - // Matches id #[RouteParam, Assert\NotBlank] public string $id; - + // Nested DTOs are supported for BodyParam and QueryParam - #[BodyParam("nested")] // Dont use Assert\Valid on nested DTOs otherwise native validation is triggered + // Do NOT use Assert\Valid here + #[BodyParam("nested")] public ?NestedRequestDTO $nestedBodyDto; - - // Optionally implement constructor which accepts request object - // Only the request is passed; properties are NOT yet initialized here. - public function __construct(Request $request) { - - } -} -``` - -> **IMPORTANT: Strict Typing**
-> While strict types are supported, validation constraint mismatches can still lead to `TypeError` in production. Always ensure your constraints (e.g., `Assert\Type`, `Assert\NotBlank`) match your property types. - -> > **IMPORTANT: Nested DTOs**
-> Dont use any assertions on nested DTO properties as this will trigger the native validation fow eventually breaking hydration and triggering errors. - -### DTO Lifecycle - -1. **Resolution**: `RequestDtoResolver` instantiates the DTO during the controller argument resolving phase. Only the `Request` object is passed to the constructor. -2. **Security**: Symfony security checks (e.g., `#[IsGranted]`) are executed. -3. **Validation & Hydration**: An event subscriber (`RequestDtoValidationEventSubscriber`) listens to `kernel.controller_arguments`. It validates the DTO data and, if successful, hydrates the DTO properties. - -### Step 2: Use the DTO in a Controller - -Inject the DTO into your controller action. It will be automatically instantiated, validated, and hydrated. -```php -namespace App\Controller; - -use App\DTO\ExampleDTO; -use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; -use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Routing\Annotation\Route; - -class ExampleController extends AbstractController -{ - #[Route("/example", name: "example")] - public function exampleAction(ExampleDTO $data): Response + // Optional constructor receiving the Request + // Properties are not initialized at this stage + public function __construct(Request $request) { - // $data is an instance of ExampleDTO with validated and hydrated request data - return new Response("DTO received and validated successfully!"); } } ``` -### Using RequestDtoTrait - -The [`RequestDtoTrait`](src/Trait/RequestDtoTrait.php) provides a default constructor that accepts the `Request` object and a `getValue(string $property)` method. This method is useful for accessing request data before hydration, which can come in handy in group sequence providers. +> **Any type mismatches will trigger a constraint violation and thus a `RequestValidationException` is thrown.** +### 1.2 Mixed typed DTO ```php -use Crtl\RequestDtoResolverBundle\Attribute\RequestDto; -use Crtl\RequestDtoResolverBundle\Trait\RequestDtoTrait; +namespace App\DTO; + use Crtl\RequestDtoResolverBundle\Attribute\BodyParam; +use Crtl\RequestDtoResolverBundle\Attribute\FileParam; +use Crtl\RequestDtoResolverBundle\Attribute\HeaderParam; +use Crtl\RequestDtoResolverBundle\Attribute\QueryParam; +use Crtl\RequestDtoResolverBundle\Attribute\RouteParam; +use Crtl\RequestDtoResolverBundle\Attribute\RequestDto; +use Symfony\Component\HttpFoundation\File\UploadedFile; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Validator\Constraints as Assert; #[RequestDto] -class MyDTO +class ExampleDTO { - use RequestDtoTrait; - - #[BodyParam] - public string $type; -} -``` - -### Validation Group Sequences - -When using [Group Sequences](https://symfony.com/doc/current/validation/sequence_provider.html) to define conditional validation, you must be careful about how you access data. - -**IMPORTANT: Uninitialized Properties** + #[BodyParam, Assert\NotBlank, Assert\Type("string")] + public string $someParam; + + /** + * @var string + */ + #[BodyParam, Assert\NotBlank, Assert\Type("string")] + public mixed $withDefaultValue = "My default value"; -Since hydration happens *after* validation, DTO properties are **uninitialized** when the group sequence is evaluated. Accessing them directly will throw an `Error`. + #[FileParam, Assert\NotNull] + public ?UploadedFile $file; -To safely access request parameters in your group sequence logic, use `RequestDtoTrait::getValue()`: + #[HeaderParam("Content-Type"), Assert\NotBlank] + public string $contentType; -```php -use Crtl\RequestDtoResolverBundle\Attribute\RequestDto; -use Crtl\RequestDtoResolverBundle\Trait\RequestDtoTrait; -use Symfony\Component\Validator\Constraints\GroupSequence; -use Symfony\Component\Validator\GroupSequenceProviderInterface; + // Because query params are all strings by default + // we can provide a type transformer to transform its type. + // values are converted using filter_var with the corrosponding FILTER_VALIDATE_* option. + #[QueryParam(name: "age", transformType: "int"), Assert\GreaterThan(18)] + public int $age; + + // Or provide a custom callable to tranform type + #[ + QueryParam( + name: "age", + transformType: fn(string $value) => strtolower($value) + ), + Assert\GreaterThan(18) + ] + public mixed $customQueryParam; -#[RequestDto] -class MyDTO implements GroupSequenceProviderInterface -{ - use RequestDtoTrait; + #[RouteParam, Assert\NotBlank] + public string $id; - #[BodyParam] - public string $type; + // Nested DTOs are supported for BodyParam and QueryParam + // Do NOT use Assert\Valid here + #[BodyParam("nested")] + public ?NestedRequestDTO $nestedBodyDto; - public function getGroupSequence(): array|GroupSequence + // Optional constructor receiving the Request + // It is recommended to make the request argument nullable to support creation of DTOs from + // array data but not required when only used in HTTP contexts. + public function __construct(?Request $request = null) { - // Use getValue() instead of $this->type - $type = $this->getValue("type"); - - $groups = ["MyDTO"]; - if ($type === "special") { - $groups[] = "Special"; - } - - return $groups; } } ``` -#### Using a Group Sequence Provider Service - -You can also use a service to provide the group sequence. This is useful if your validation logic depends on external services (e.g., a database or configuration). - -1. **Create the Provider Service**: +### Step 2: Use the DTO in a Controller ```php -namespace App\Validator; +namespace App\Controller; -use App\DTO\MyDTO; -use Symfony\Component\Validator\Constraints\GroupSequence; -use Symfony\Component\Validator\GroupProviderInterface; +use App\DTO\ExampleDTO; +use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Routing\Annotation\Route; -class MyGroupSequenceProvider implements GroupProviderInterface +class ExampleController extends AbstractController { - public function getGroups(object $object): array|GroupSequence + #[Route("/example", name: "example")] + public function exampleAction(ExampleDTO $data): Response { - assert($object instanceof MyDTO) - - $groups = ["MyDTO"]; - - // Use getValue() to safely access uninitialized properties - if ($object->getValue("type") === "special") { - $groups[] = "Special"; - } - - return $groups; + return new Response("DTO received and validated successfully!"); } } ``` -2. **Configure the DTO**: +### DTO Lifecycle -```php -use App\Validator\MyGroupSequenceProvider; -use Crtl\RequestDtoResolverBundle\Attribute\RequestDto; -use Crtl\RequestDtoResolverBundle\Trait\RequestDtoTrait; -use Symfony\Component\Validator\Constraints as Assert; +1. **Resolution** + `RequestDtoResolver` instantiates the DTO during controller argument resolving. -#[RequestDto] -#[Assert\GroupSequenceProvider(provider: MyGroupSequenceProvider::class)] -class MyDTO -{ - // Trait is important to access fields before validation - use RequestDtoTrait; - - // ... -} -``` +2. **Security** + Symfony security checks (e.g. `#[IsGranted]`) are executed. + +3. **Validation** + An event subscriber validates the DTO and hydrates its properties if validation passes, otherwise an `Crtl\RequestDtoResolverBundle\Exception\RequestValidationException` is thrown. -> **Note**: `getValue()` only works in **root DTOs**. It uses reflection to resolve data from the request, which cannot access parent data in nested contexts. +### Validation Group Sequences + +Though all variations of group sequence providers are supported you still have +to consider unitialized properties when using strict types because of invalid input. +Make sure to ensure properties are initialized using `isset()` or reflection. -### Step 3: Handle Validation Errors +### Handling Validation Errors -When validation fails, a [`Crtl\RequestDtoResolverBundle\Exception\RequestValidationException`](src/Exception/RequestValidationException.php) is thrown. +On validation failure, a `RequestValidationException` is thrown. -The bundle registers a default exception subscriber ([`RequestValidationExceptionEventSubscriber`](src/EventSubscriber/RequestValidationExceptionEventSubscriber.php)) with a low priority of **-32**. This ensures that validation exceptions are caught and converted into a `JsonResponse` with a `400 Bad Request` status code by default. +> **The bundle registers a default exception subscriber (priority **-32**) that +returns a `400 Bad Request` JSON response.** -You can still provide your own listener if you need custom error formatting: +You can override this with your own listener: ```php namespace App\EventListener; @@ -261,7 +231,6 @@ class RequestValidationExceptionListener implements EventSubscriberInterface public static function getSubscribedEvents(): array { return [ - // Use a priority > -32 to override the default bundle subscriber KernelEvents::EXCEPTION => ["onKernelException", 0], ]; } @@ -271,12 +240,10 @@ class RequestValidationExceptionListener implements EventSubscriberInterface $exception = $event->getThrowable(); if ($exception instanceof RequestValidationException) { - $response = new JsonResponse([ + $event->setResponse(new JsonResponse([ "error" => "Validation failed", "details" => $exception->getViolations(), - ], JsonResponse::HTTP_BAD_REQUEST); - - $event->setResponse($response); + ], JsonResponse::HTTP_BAD_REQUEST)); } } } @@ -284,4 +251,4 @@ class RequestValidationExceptionListener implements EventSubscriberInterface ## License -This bundle is licensed under the MIT License. See the [LICENSE](LICENSE) file for more details. +This bundle is licensed under the MIT License. See the [LICENSE](LICENSE) file for details. \ No newline at end of file diff --git a/src/Attribute/AbstractNestedParam.php b/src/Attribute/AbstractNestedParam.php index d9dd81a..4c712c5 100644 --- a/src/Attribute/AbstractNestedParam.php +++ b/src/Attribute/AbstractNestedParam.php @@ -43,6 +43,20 @@ protected function getDataFromRequest(Request $request): array return $this->getInputBag($request)->all(); } + public function hasValueInRequest(Request $request): bool + { + if (false === $this->parent?->hasValueInRequest($request)) { + return false; + } + + $data = $this->parent + ? $this->parent->getValueFromRequest($request) + : $this->getDataFromRequest($request) + ; + + return array_key_exists($this->getName(), $data); + } + public function getValueFromRequest(Request $request): mixed { $name = $this->getName(); diff --git a/src/Attribute/AbstractParam.php b/src/Attribute/AbstractParam.php index f086a65..714f911 100644 --- a/src/Attribute/AbstractParam.php +++ b/src/Attribute/AbstractParam.php @@ -83,4 +83,9 @@ public function getName(): string * Retrieves the value from the request and returns it or null if no values was found. */ abstract public function getValueFromRequest(Request $request): mixed; + + /** + * Whether or not the request includes the value. + */ + abstract public function hasValueInRequest(Request $request): bool; } diff --git a/src/Attribute/FileParam.php b/src/Attribute/FileParam.php index b8fe9e6..8ade296 100644 --- a/src/Attribute/FileParam.php +++ b/src/Attribute/FileParam.php @@ -23,6 +23,11 @@ #[\Attribute(\Attribute::TARGET_PROPERTY)] class FileParam extends AbstractParam { + public function hasValueInRequest(Request $request): bool + { + return $request->files->has($this->getName()); + } + /** * @throws \UnexpectedValueException When FileBag::get() does not return null or `UploadedFile` instance */ diff --git a/src/Attribute/HeaderParam.php b/src/Attribute/HeaderParam.php index 0ed86fd..40e6bcc 100644 --- a/src/Attribute/HeaderParam.php +++ b/src/Attribute/HeaderParam.php @@ -22,6 +22,11 @@ #[\Attribute(\Attribute::TARGET_PROPERTY)] class HeaderParam extends AbstractParam { + public function hasValueInRequest(Request $request): bool + { + return $request->headers->has($this->getName()); + } + public function getValueFromRequest(Request $request): ?string { return $request->headers->get($this->getName()); diff --git a/src/Attribute/RouteParam.php b/src/Attribute/RouteParam.php index 2de2c95..3b87341 100644 --- a/src/Attribute/RouteParam.php +++ b/src/Attribute/RouteParam.php @@ -22,6 +22,17 @@ #[\Attribute(\Attribute::TARGET_PROPERTY)] class RouteParam extends AbstractParam { + public function hasValueInRequest(Request $request): bool + { + $key = '_route_params'; + + return $request->attributes->has($key) + && array_key_exists( + $this->getName(), + $request->attributes->get($key, []), + ); + } + public function getValueFromRequest(Request $request): mixed { /** @var array $routeParams */ diff --git a/src/Factory/RequestDtoFactory.php b/src/Factory/RequestDtoFactory.php index c84fdb3..afaf3fd 100644 --- a/src/Factory/RequestDtoFactory.php +++ b/src/Factory/RequestDtoFactory.php @@ -107,7 +107,8 @@ public function fromArray(string $className, array $data): object return $this->createInstanceRecursive( $className, $data, - fn (AbstractParam $attr, RequestDtoParamMetadata $paramMetadata, array $data) => $data[$paramMetadata->getPropertyName()] ?? null, + [$this, 'arrayValueProvider'], + [$this, 'arrayValueChecker'], ); } @@ -133,7 +134,8 @@ public function fromRequest(string $className, Request $request): object return $this->createInstanceRecursive( $className, $request, - fn (AbstractParam $attr, RequestDtoParamMetadata $paramMetadata, Request $request) => $attr->getValueFromRequest($request), + [$this, 'requestValueProvider'], + [$this, 'requestValueChecker'], ); } @@ -146,6 +148,7 @@ public function fromRequest(string $className, Request $request): object * @param class-string $className DTO class name to instantiate * @param TContext $context data source used to resolve values * @param callable(AbstractParam, RequestDtoParamMetadata, TContext): mixed $valueProvider resolves a raw value for a single property + * @param callable(AbstractParam, RequestDtoParamMetadata, TContext): bool $valueChecker checks whether value exists in context, values are only assigned if valueChecker returns true * @param AbstractParam|null $parentAttribute parent attribute for nested hydration * @param RequestDtoMetadata|null $metadata pre-resolved metadata for recursion reuse * @param string[] $stack DTO class stack used for circular reference detection @@ -160,6 +163,7 @@ private function createInstanceRecursive( string $className, Request|array $context, callable $valueProvider, + callable $valueChecker, ?AbstractParam $parentAttribute = null, ?RequestDtoMetadata $metadata = null, array $stack = [], @@ -193,13 +197,14 @@ private function createInstanceRecursive( } } - $requestValue = $valueProvider($attr, $propertyMetadata, $context); - - $value = $requestValue - ?? $propertyMetadata->getDefaultValue(); + $hasValue = $valueChecker($attr, $propertyMetadata, $context); + $value = null; + if ($hasValue) { + $value = $valueProvider($attr, $propertyMetadata, $context); + } // Property is typed with nested dto - if (null !== $nestedClassName && null !== $value) { + if (null !== $nestedClassName && $hasValue && null !== $value) { $isArray = $propertyMetadata->isNestedDtoArray(); /** @var class-string $nestedClassName */ @@ -224,6 +229,7 @@ private function createInstanceRecursive( ? $context : $nestedValue, $valueProvider, + $valueChecker, $nestedAttr, $nestedMetadata, $stack, @@ -248,6 +254,14 @@ private function createInstanceRecursive( $value = $isArray ? $resultArray : $resultArray[0]; } + if (is_null($value) && $propertyMetadata->isNullable()) { + continue; + } + + if (!$hasValue) { + continue; + } + try { $this->assignProperty($object, $propertyMetadata, $value); } catch (PropertyHydrationException $e) { @@ -358,4 +372,42 @@ private function prefixViolation(ConstraintViolationInterface $violation, string $violation->getCause(), ); } + + /** + * @param array $data + */ + private function arrayValueProvider( + AbstractParam $attr, + RequestDtoParamMetadata $paramMetadata, + array $data + ): mixed { + return $data[$paramMetadata->getPropertyName()] ?? null; + } + + /** + * @param array $data + */ + private function arrayValueChecker( + AbstractParam $attr, + RequestDtoParamMetadata $paramMetadata, + array $data + ): bool { + return array_key_exists($paramMetadata->getPropertyName(), $data); + } + + private function requestValueProvider( + AbstractParam $attr, + RequestDtoParamMetadata $paramMetadata, + Request $request, + ): mixed { + return $attr->getValueFromRequest($request); + } + + private function requestValueChecker( + AbstractParam $attr, + RequestDtoParamMetadata $paramMetadata, + Request $request, + ): bool { + return $attr->hasValueInRequest($request); + } } diff --git a/src/Reflection/RequestDtoParamMetadata.php b/src/Reflection/RequestDtoParamMetadata.php index 37b0347..611abd4 100644 --- a/src/Reflection/RequestDtoParamMetadata.php +++ b/src/Reflection/RequestDtoParamMetadata.php @@ -124,6 +124,11 @@ public function isNullable(): bool return $this->isNullable; } + public function hasDefaultValue(): bool + { + return $this->getReflectionProperty()->hasDefaultValue(); + } + public function getDefaultValue(): mixed { return $this->getReflectionProperty()->getDefaultValue(); diff --git a/test/Integration/RequestDtoFactoryIntegrationTest.php b/test/Integration/RequestDtoFactoryIntegrationTest.php index 8954b41..22e87c8 100644 --- a/test/Integration/RequestDtoFactoryIntegrationTest.php +++ b/test/Integration/RequestDtoFactoryIntegrationTest.php @@ -143,12 +143,11 @@ public function testFromArrayThrowsExceptionWithNestedArrayRequestDtoTypeMismatc ]], ); } catch (RequestDtoHydrationException $e) { - $this->assertCount(5, $e->violations); - self::assertSame('children[0].childName', $e->violations[0]->getPropertyPath()); - self::assertSame('children[1].childName', $e->violations[1]->getPropertyPath()); - self::assertSame('children[2].childName', $e->violations[2]->getPropertyPath()); - self::assertSame('children[3].childName', $e->violations[3]->getPropertyPath()); - self::assertSame('children[4].childName', $e->violations[4]->getPropertyPath()); + $this->assertCount(4, $e->violations); + self::assertSame('children[1].childName', $e->violations[0]->getPropertyPath()); + self::assertSame('children[2].childName', $e->violations[1]->getPropertyPath()); + self::assertSame('children[3].childName', $e->violations[2]->getPropertyPath()); + self::assertSame('children[4].childName', $e->violations[3]->getPropertyPath()); throw $e; } diff --git a/test/Integration/RequestDtoResolverBundleIntegrationTest.php b/test/Integration/RequestDtoResolverBundleIntegrationTest.php index 1a0aa18..56ecb7e 100644 --- a/test/Integration/RequestDtoResolverBundleIntegrationTest.php +++ b/test/Integration/RequestDtoResolverBundleIntegrationTest.php @@ -603,7 +603,13 @@ public function testTypeConflictingDtoReturnsValidationErrorsWhenPropertyTypeMis server: [ 'CONTENT_TYPE' => 'application/json', ], - content: json_encode([], JSON_THROW_ON_ERROR), + content: json_encode([ + 'arrayProperty' => 'string', + 'intProperty' => 'string', + 'floatProperty' => 'string', + 'boolProperty' => 'string', + 'stringProperty' => 1 + ], JSON_THROW_ON_ERROR), ); $controller = new class { diff --git a/test/Unit/Attribute/AbstractParamTest.php b/test/Unit/Attribute/AbstractParamTest.php index 0a16dcb..0c9bef9 100644 --- a/test/Unit/Attribute/AbstractParamTest.php +++ b/test/Unit/Attribute/AbstractParamTest.php @@ -24,6 +24,11 @@ final class TestClass final class TestParam extends AbstractParam { + public function hasValueInRequest(Request $request): bool + { + return $request->request->has($this->getName()); + } + public function getValueFromRequest(Request $request): mixed { return $request->request->get($this->getName()); From dd77af082cea6f0de0b3efca4f6fe0982159ce38 Mon Sep 17 00:00:00 2001 From: Marvin Petker Date: Sun, 8 Feb 2026 04:12:34 +0100 Subject: [PATCH 3/6] docs: update readme --- README.md | 123 +++++++++--------- phpunit.xml.dist | 12 +- src/Reflection/RequestDtoMetadata.php | 50 +------ src/Reflection/RequestDtoMetadataFactory.php | 16 +-- .../RequestDtoParamMetadataFactory.php | 5 +- src/Utility/DtoInstanceBagInterface.php | 3 + test/Unit/Attribute/BodyParamTest.php | 44 +++++++ test/Unit/Attribute/FileParamTest.php | 15 ++- test/Unit/Attribute/HeaderParamTest.php | 15 ++- test/Unit/Attribute/QueryParamTest.php | 44 +++++++ test/Unit/Attribute/RouteParamTest.php | 18 ++- .../RequestDtoMetadataFactoryTest.php | 16 ++- .../Reflection/RequestDtoMetadataTest.php | 110 +--------------- .../RequestDtoParamMetadataTest.php | 54 ++++++++ test/Unit/Utility/DtoReflectionHelperTest.php | 7 + 15 files changed, 282 insertions(+), 250 deletions(-) diff --git a/README.md b/README.md index cd579b3..c682f3b 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ Use parameter attributes to map request data to properties. > **The attribute is required to identify which controller arguments should be resolved and validated.** -### 1.1 Strictly typed DTO +#### 1.1 Strictly typed DTO ```php namespace App\DTO; @@ -74,10 +74,16 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Validator\Constraints as Assert; #[RequestDto] -class ExampleDTO +class ExampleDto { #[BodyParam, Assert\NotBlank, Assert\Type("string")] public string $someParam; + + #[BodyParam, Assert\NotBlank, Assert\Type("string")] + public string $withDefaultValue = "My default value"; + + #[BodyParam] + public ?string $nullable; #[FileParam, Assert\NotNull] public ?UploadedFile $file; @@ -85,8 +91,21 @@ class ExampleDTO #[HeaderParam("Content-Type"), Assert\NotBlank] public string $contentType; - #[QueryParam(name: "age", transformType: "int"), Assert\GreaterThan(18)] + // Because query params are all strings by default + // we can provide a type transformer to transform its type. + // values are converted using filter_var with the corrosponding FILTER_VALIDATE_* option. + #[QueryParam(transformType: "int"), Assert\GreaterThan(18)] public int $age; + + // Or provide a custom callable to tranform type + #[ + QueryParam( + name: "age", + transformType: fn(string $value) => strtolower($value) + ), + Assert\GreaterThan(18) + ] + public mixed $customQueryParam; #[RouteParam, Assert\NotBlank] public string $id; @@ -97,8 +116,9 @@ class ExampleDTO public ?NestedRequestDTO $nestedBodyDto; // Optional constructor receiving the Request - // Properties are not initialized at this stage - public function __construct(Request $request) + // It is recommended to make the request argument nullable to support creation of DTOs from + // array data but not required when only used in HTTP contexts. + public function __construct(?Request $request = null) { } } @@ -106,7 +126,7 @@ class ExampleDTO > **Any type mismatches will trigger a constraint violation and thus a `RequestValidationException` is thrown.** -### 1.2 Mixed typed DTO +#### 1.2 Mixed typed DTO ```php namespace App\DTO; @@ -121,62 +141,52 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Validator\Constraints as Assert; #[RequestDto] -class ExampleDTO +class ExampleDto { - #[BodyParam, Assert\NotBlank, Assert\Type("string")] - public string $someParam; - /** - * @var string + * @var string */ - #[BodyParam, Assert\NotBlank, Assert\Type("string")] - public mixed $withDefaultValue = "My default value"; + #[BodyParam, Assert\NotBlank] + public mixed $contentType; +} +``` - #[FileParam, Assert\NotNull] - public ?UploadedFile $file; +### 1.3 Important notes about DTO hydration - #[HeaderParam("Content-Type"), Assert\NotBlank] - public string $contentType; +- **Uninitialized properties** + If a request does not contain data for a property, that property will remain uninitialized. + To guarantee initialization, either: + - provide a default value, or + - add appropriate validation constraints (e.g. `NotNull`, `NotBlank`). - // Because query params are all strings by default - // we can provide a type transformer to transform its type. - // values are converted using filter_var with the corrosponding FILTER_VALIDATE_* option. - #[QueryParam(name: "age", transformType: "int"), Assert\GreaterThan(18)] - public int $age; - - // Or provide a custom callable to tranform type - #[ - QueryParam( - name: "age", - transformType: fn(string $value) => strtolower($value) - ), - Assert\GreaterThan(18) - ] - public mixed $customQueryParam; +- **Hydration from arrays** + When a DTO is manually hydrated from an array using `RequestDtoFactory::fromArray()`, the configured `AbstractParam::$name` is ignored. + In this case, the DTO’s property names are always used as the source keys. - #[RouteParam, Assert\NotBlank] - public string $id; +- **Validation still runs in strict mode** + Even if some properties cannot be assigned due to type mismatches in strict typing mode, the DTO is still fully validated using Symfony’s validator. + Any resulting violations will lead to a `RequestValidationException`. - // Nested DTOs are supported for BodyParam and QueryParam - // Do NOT use Assert\Valid here - #[BodyParam("nested")] - public ?NestedRequestDTO $nestedBodyDto; +### 1.4 Validation group sequences + +All Symfony validation group sequence variants are supported. + +Because request data can never be trusted, **DTO properties may be uninitialized regardless of whether strict typing is used or not**. +Missing or invalid input can prevent a property from being assigned during hydration. + +When using validation group sequences, you must therefore ensure that properties are accessed safely by: +- checking initialization with `isset()`, or +- using reflection-based checks when necessary. + +Failing to do so may lead to runtime errors before later validation groups are evaluated. - // Optional constructor receiving the Request - // It is recommended to make the request argument nullable to support creation of DTOs from - // array data but not required when only used in HTTP contexts. - public function __construct(?Request $request = null) - { - } -} -``` ### Step 2: Use the DTO in a Controller ```php namespace App\Controller; -use App\DTO\ExampleDTO; +use App\DTO\ExampleDto; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; @@ -184,29 +194,14 @@ use Symfony\Component\Routing\Annotation\Route; class ExampleController extends AbstractController { #[Route("/example", name: "example")] - public function exampleAction(ExampleDTO $data): Response + public function exampleAction(ExampleDto $data): Response { return new Response("DTO received and validated successfully!"); } } ``` -### DTO Lifecycle - -1. **Resolution** - `RequestDtoResolver` instantiates the DTO during controller argument resolving. - -2. **Security** - Symfony security checks (e.g. `#[IsGranted]`) are executed. - -3. **Validation** - An event subscriber validates the DTO and hydrates its properties if validation passes, otherwise an `Crtl\RequestDtoResolverBundle\Exception\RequestValidationException` is thrown. - -### Validation Group Sequences - -Though all variations of group sequence providers are supported you still have -to consider unitialized properties when using strict types because of invalid input. -Make sure to ensure properties are initialized using `isset()` or reflection. +--- ### Handling Validation Errors diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 8621091..2d95056 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -3,11 +3,7 @@ xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.5/phpunit.xsd" colors="true" cacheDirectory=".phpunit.cache" - executionOrder="depends,defects" beStrictAboutCoverageMetadata="true" - beStrictAboutOutputDuringTests="false" - failOnRisky="false" - failOnWarning="false" bootstrap="test/bootstrap.php" > @@ -20,8 +16,12 @@ - - test + + test/Unit + + + + test/Integration diff --git a/src/Reflection/RequestDtoMetadata.php b/src/Reflection/RequestDtoMetadata.php index 757d622..d0207cd 100644 --- a/src/Reflection/RequestDtoMetadata.php +++ b/src/Reflection/RequestDtoMetadata.php @@ -24,11 +24,6 @@ class RequestDtoMetadata */ private array $propertyMetadata = []; - /** - * @var array - */ - private array $constrainedProperties = []; - /** * @param RequestDtoParamMetadata[] $propertyMetadata */ @@ -40,7 +35,7 @@ public function __construct( */ private readonly string $className, - /* + /** * Property names mapped to metadata * * @var RequestDtoParamMetadata[] @@ -57,10 +52,6 @@ public function __construct( foreach ($propertyMetadata as $propMetadata) { $propertyName = $propMetadata->getPropertyName(); $this->propertyMetadata[$propertyName] = $propMetadata; - - if ($propMetadata->isConstrained()) { - $this->constrainedProperties[$propertyName] = $propMetadata; - } } } @@ -121,19 +112,6 @@ public function getReflectionClass(): \ReflectionClass return new \ReflectionClass($this->className); } - /** - * @return \Symfony\Component\Validator\Constraint[] - */ - public function getClassConstraints(): array - { - return $this->validatorMetadata->getConstraints(); - } - - public function getAbstractParamAttributeFromProperty(\ReflectionProperty $property, ?AbstractParam $parent = null): ?AbstractParam - { - return $this->propertyMetadata[$property->getName()]->getAttribute($parent); - } - public function getPropertyMetadataGenerator(): \Generator { foreach ($this->propertyMetadata as $name => $property) { @@ -141,32 +119,6 @@ public function getPropertyMetadataGenerator(): \Generator } } - public function getValidatorMetadata(): ClassMetadataInterface - { - return $this->validatorMetadata; - } - - /** - * @return array|null - */ - public function getGroupSequence(): ?array - { - $sequence = $this->validatorMetadata->getGroupSequence(); - if ($sequence instanceof GroupSequence) { - return $sequence->groups; - } - - return $sequence; - } - - public function isConstrainedProperty(string|\ReflectionProperty $propertyName): bool - { - if ($propertyName instanceof \ReflectionProperty) { - $propertyName = $propertyName->getName(); - } - - return array_key_exists($propertyName, $this->constrainedProperties); - } /** * @param mixed ...$args Arguments passed to new instance constructor, only if implemented diff --git a/src/Reflection/RequestDtoMetadataFactory.php b/src/Reflection/RequestDtoMetadataFactory.php index 2312e80..c3444d2 100644 --- a/src/Reflection/RequestDtoMetadataFactory.php +++ b/src/Reflection/RequestDtoMetadataFactory.php @@ -15,6 +15,7 @@ use Crtl\RequestDtoResolverBundle\Utility\DtoReflectionHelper; use Psr\Cache\CacheItemPoolInterface; +use Psr\Cache\InvalidArgumentException; use Symfony\Component\Validator\Mapping\ClassMetadataInterface; use Symfony\Component\Validator\Validator\ValidatorInterface; @@ -33,6 +34,9 @@ public function __construct( /** * @param class-string $className + * @return RequestDtoMetadata + * @throws InvalidArgumentException + * @throws \ReflectionException */ public function getMetadataFor(string $className): RequestDtoMetadata { @@ -58,21 +62,9 @@ public function getMetadataFor(string $className): RequestDtoMetadata $propertyMetadata[] = $this->requestDtoParamMetadataFactory->getMetadataFor($property); } - // // TODO: Think about a better way of liniting because this only works on the first level - // // due to nested metadata only being instantiated on hydration. - // foreach ($properties as $property) { - // // Ensure no union or intersection types are used for nested dto params - // $this->reflectionHelper->getDtoClassNameFromReflectionProperty($property); - // } - - // $constrainedPropertyNames = $validatorMetadata->getConstrainedProperties(); - - // $propertyNames = array_map(fn (\ReflectionProperty $prop) => $prop->getName(), $properties); $metadata = new RequestDtoMetadata( $className, $propertyMetadata, - // $propertyNames, - // array_intersect($constrainedPropertyNames, $propertyNames), $validatorMetadata, ); diff --git a/src/Reflection/RequestDtoParamMetadataFactory.php b/src/Reflection/RequestDtoParamMetadataFactory.php index b9e5972..6e3ef2d 100644 --- a/src/Reflection/RequestDtoParamMetadataFactory.php +++ b/src/Reflection/RequestDtoParamMetadataFactory.php @@ -26,8 +26,8 @@ class RequestDtoParamMetadataFactory { public function __construct( - private ValidatorInterface $validator, - private DtoReflectionHelper $reflectionHelper, + private readonly ValidatorInterface $validator, + private readonly DtoReflectionHelper $reflectionHelper, private readonly PropertyInfoExtractorInterface $propertyInfoExtractor, ) { } @@ -35,7 +35,6 @@ public function __construct( public function getMetadataFor(\ReflectionProperty $property): RequestDtoParamMetadata { $className = $property->getDeclaringClass()->getName(); - $cacheKey = $className.'.'.$property->getName(); /** @var ClassMetadataInterface $validatorClassMetadata */ $validatorClassMetadata = $this->validator->getMetadataFor($className); diff --git a/src/Utility/DtoInstanceBagInterface.php b/src/Utility/DtoInstanceBagInterface.php index e8c1faf..0d07871 100644 --- a/src/Utility/DtoInstanceBagInterface.php +++ b/src/Utility/DtoInstanceBagInterface.php @@ -16,6 +16,9 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Validator\ConstraintViolationListInterface; +/** + * @codeCoverageIgnore + */ interface DtoInstanceBagInterface { /** diff --git a/test/Unit/Attribute/BodyParamTest.php b/test/Unit/Attribute/BodyParamTest.php index 934df8b..3684cf8 100644 --- a/test/Unit/Attribute/BodyParamTest.php +++ b/test/Unit/Attribute/BodyParamTest.php @@ -14,6 +14,7 @@ namespace Crtl\RequestDtoResolverBundle\Test\Unit\Attribute; use Crtl\RequestDtoResolverBundle\Attribute\BodyParam; +use Crtl\RequestDtoResolverBundle\Attribute\QueryParam; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; @@ -68,4 +69,47 @@ public function testReadsValueFromJsonRequest(): void self::assertSame('John Doe', $value); } + + public function testHasValueInRequestReturnsWhetherValueIsExistsInRequest(): void + { + $request = new Request(request: ["param" => "value"]); + + $param = new BodyParam("param"); + + $this->assertTrue($param->hasValueInRequest($request)); + $this->assertFalse($param->hasValueInRequest(new Request())); + } + + public function testHasValueInRequestWithNestedParam(): void + { + $parent = new BodyParam('parent'); + $child = new BodyParam('child'); + $request = new Request(request: [ + "parent" => [ + "child" => "value", + ] + ]); + + $child->setParent($parent); + + $this->assertTrue($child->hasValueInRequest($request)); + $this->assertFalse($child->hasValueInRequest(new Request())); + } + + public function testHasValueInRequestWithNestedArrayParam(): void + { + $parent = new BodyParam('parent'); + $child = new BodyParam('child'); + $request = new Request(request: [ + "parent" => [ + ["child" => "value",] + ] + ]); + + $parent->setIndex(0); + $child->setParent($parent); + + $this->assertTrue($child->hasValueInRequest($request)); + $this->assertFalse($child->hasValueInRequest(new Request())); + } } diff --git a/test/Unit/Attribute/FileParamTest.php b/test/Unit/Attribute/FileParamTest.php index 95a9b3c..7575de1 100644 --- a/test/Unit/Attribute/FileParamTest.php +++ b/test/Unit/Attribute/FileParamTest.php @@ -29,7 +29,7 @@ public function testGetValueFromRequestReturnsUploadedFileFromRequestFilesBag(): $paramName = 'test_file'; $uploadedFile = $this->createMock(UploadedFile::class); - $request = new Request([], [], [], [], [$paramName => $uploadedFile]); + $request = new Request(files: [$paramName => $uploadedFile]); $fileParam = new FileParam($paramName); @@ -46,4 +46,17 @@ public function testGetValueFromRequestReturnsNullIfFileIsMissing(): void $this->assertNull($fileParam->getValueFromRequest($request)); } + + public function testHasValueInRequestReturnsWhetherValueIsExistsInRequest(): void + { + $paramName = 'test_file'; + $uploadedFile = $this->createMock(UploadedFile::class); + + $request = new Request(files: [$paramName => $uploadedFile]); + + $fileParam = new FileParam($paramName); + + $this->assertTrue($fileParam->hasValueInRequest($request)); + $this->assertFalse($fileParam->hasValueInRequest(new Request())); + } } diff --git a/test/Unit/Attribute/HeaderParamTest.php b/test/Unit/Attribute/HeaderParamTest.php index 948288d..ae6fe98 100644 --- a/test/Unit/Attribute/HeaderParamTest.php +++ b/test/Unit/Attribute/HeaderParamTest.php @@ -24,7 +24,7 @@ public function testGetValueFromRequestReturnsHeaderValueFromRequestHeadersBag() $paramName = 'test_header'; $paramValue = 'test_value'; - $request = new Request([], [], [], [], [], ['HTTP_'.strtoupper($paramName) => $paramValue]); + $request = new Request(server: ['HTTP_'.strtoupper($paramName) => $paramValue]); $headerParam = new HeaderParam($paramName); @@ -41,4 +41,17 @@ public function testGetValueFromRequestReturnsNullIfHeaderIsMissing(): void $this->assertNull($headerParam->getValueFromRequest($request)); } + + public function testHasValueInRequestReturnsWhetherValueIsExistsInRequest(): void + { + $paramName = 'test_header'; + $paramValue = 'test_value'; + + $request = new Request(server: ['HTTP_'.strtoupper($paramName) => $paramValue]); + + $param = new HeaderParam($paramName); + + $this->assertTrue($param->hasValueInRequest($request)); + $this->assertFalse($param->hasValueInRequest(new Request())); + } } diff --git a/test/Unit/Attribute/QueryParamTest.php b/test/Unit/Attribute/QueryParamTest.php index 7d5cb6e..e8a2101 100644 --- a/test/Unit/Attribute/QueryParamTest.php +++ b/test/Unit/Attribute/QueryParamTest.php @@ -13,6 +13,7 @@ namespace Crtl\RequestDtoResolverBundle\Test\Unit\Attribute; +use Crtl\RequestDtoResolverBundle\Attribute\HeaderParam; use Crtl\RequestDtoResolverBundle\Attribute\QueryParam; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -100,4 +101,47 @@ public function testGetValueFromRequestReturnsArrayWithoutTransformationIfValueI $queryParam = new QueryParam('ids', 'int'); $this->assertSame(['1', '2'], $queryParam->getValueFromRequest($request)); } + + public function testHasValueInRequestReturnsWhetherValueIsExistsInRequest(): void + { + $request = new Request(["param" => "value"], [], [], [], [], []); + + $param = new QueryParam("param"); + + $this->assertTrue($param->hasValueInRequest($request)); + $this->assertFalse($param->hasValueInRequest(new Request())); + } + + public function testHasValueInRequestWithNestedParam(): void + { + $parent = new QueryParam('parent'); + $child = new QueryParam('child'); + $request = new Request([ + "parent" => [ + "child" => "value", + ] + ]); + + $child->setParent($parent); + + $this->assertTrue($child->hasValueInRequest($request)); + $this->assertFalse($child->hasValueInRequest(new Request())); + } + + public function testHasValueInRequestWithNestedArrayParam(): void + { + $parent = new QueryParam('parent'); + $child = new QueryParam('child'); + $request = new Request([ + "parent" => [ + ["child" => "value",] + ] + ]); + + $parent->setIndex(0); + $child->setParent($parent); + + $this->assertTrue($child->hasValueInRequest($request)); + $this->assertFalse($child->hasValueInRequest(new Request())); + } } diff --git a/test/Unit/Attribute/RouteParamTest.php b/test/Unit/Attribute/RouteParamTest.php index 8a9638a..dfd6bcd 100644 --- a/test/Unit/Attribute/RouteParamTest.php +++ b/test/Unit/Attribute/RouteParamTest.php @@ -13,6 +13,7 @@ namespace Crtl\RequestDtoResolverBundle\Test\Unit\Attribute; +use Crtl\RequestDtoResolverBundle\Attribute\HeaderParam; use Crtl\RequestDtoResolverBundle\Attribute\RouteParam; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; @@ -24,7 +25,7 @@ public function testGetValueFromRequestReturnsRouteParameterFromRequestAttribute $paramName = 'test_route'; $paramValue = 'test_value'; - $request = new Request([], [], ['_route_params' => [$paramName => $paramValue]]); + $request = new Request(attributes: ['_route_params' => [$paramName => $paramValue]]); $routeParam = new RouteParam($paramName); @@ -35,7 +36,7 @@ public function testGetValueFromRequestReturnsNullIfRouteParameterIsMissing(): v { $paramName = 'missing_route'; - $request = new Request([], [], ['_route_params' => []]); + $request = new Request(attributes: ['_route_params' => []]); $routeParam = new RouteParam($paramName); @@ -52,4 +53,17 @@ public function testGetValueFromRequestReturnsNullIfNoRouteParametersArePresent( $this->assertNull($routeParam->getValueFromRequest($request)); } + + public function testHasValueInRequestReturnsWhetherValueIsExistsInRequest(): void + { + $paramName = 'test_route'; + $paramValue = 'test_value'; + + $request = new Request(attributes: ['_route_params' => [$paramName => $paramValue]]); + + $param = new RouteParam($paramName); + + $this->assertTrue($param->hasValueInRequest($request)); + $this->assertFalse($param->hasValueInRequest(new Request())); + } } diff --git a/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php b/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php index dd63388..5f91c4f 100644 --- a/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php +++ b/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php @@ -70,9 +70,6 @@ public function testGetMetadataForReturnsRequestDtoMetadataForGivenClassName(): $this->assertEquals($className, $metadata->getReflectionClass()->getName()); $this->assertEquals(['prop1', 'prop2'], array_keys(iterator_to_array($metadata->getPropertyMetadataGenerator()))); - $this->assertTrue($metadata->isConstrainedProperty('prop1')); - $this->assertFalse($metadata->isConstrainedProperty('prop2')); - $this->assertSame($validatorMetadata, $metadata->getValidatorMetadata()); } public function testGetMetadataForReturnsCachedMetadataOnCacheHit(): void @@ -170,6 +167,12 @@ public function testGetMetadataForThrowsRuntimeExceptionWhenUnsupportedTypeIsEnc $this->factory->getMetadataFor($className); } + + public function testGetMetadataForThrowsLogicExceptionWhenClassIsNotInstantiable(): void + { + $this->expectException(\LogicException::class); + $this->factory->getMetadataFor(PrivateConstructor::class); + } } final class DummyDto @@ -178,3 +181,10 @@ final class DummyDto public string $prop2; } + +final class PrivateConstructor { + private function __construct() + { + + } +} \ No newline at end of file diff --git a/test/Unit/Reflection/RequestDtoMetadataTest.php b/test/Unit/Reflection/RequestDtoMetadataTest.php index a6fe1c1..6e1bde5 100644 --- a/test/Unit/Reflection/RequestDtoMetadataTest.php +++ b/test/Unit/Reflection/RequestDtoMetadataTest.php @@ -38,72 +38,10 @@ public function testMetadataAccessorsReturnCorrectValues(): void ); $this->assertCount(2, iterator_to_array($metadata->getPropertyMetadataGenerator())); - $this->assertSame($validatorMetadata, $metadata->getValidatorMetadata()); + $this->assertSame(DummyRequestDto::class, $metadata->getClassName()); $this->assertEquals(DummyRequestDto::class, $metadata->getReflectionClass()->getName()); } - public function testIsConstrainedPropertyReturnsTrueIfPropertyHasConstraints(): void - { - $metadata = new RequestDtoMetadata( - DummyRequestDto::class, - [ - new RequestDtoParamMetadata(DummyRequestDto::class, 'prop1', 'mixed', true), - new RequestDtoParamMetadata(DummyRequestDto::class, 'prop2', 'mixed', false), - ], - $this->createMock(ClassMetadataInterface::class), - ); - - $this->assertTrue($metadata->isConstrainedProperty('prop1')); - $this->assertTrue($metadata->isConstrainedProperty($metadata->getPropertyMetadata('prop1')->getReflectionProperty())); - - $this->assertFalse($metadata->isConstrainedProperty('prop2')); - $this->assertFalse($metadata->isConstrainedProperty($metadata->getPropertyMetadata('prop2')->getReflectionProperty())); - - $this->assertFalse($metadata->isConstrainedProperty('nonExistent')); - } - - public function testGetGroupSequenceReturnsArrayIfItIsSetAsArrayInValidatorMetadata(): void - { - $validatorMetadata = $this->createMock(ClassMetadataInterface::class); - $validatorMetadata->method('getGroupSequence')->willReturn(null); - - $metadata = new RequestDtoMetadata( - DummyRequestDto::class, - [], - $validatorMetadata, - ); - - $this->assertNull($metadata->getGroupSequence()); - } - - public function testGetGroupSequenceReturnsArrayFromGroupSequenceObjectInValidatorMetadata(): void - { - $groupSequence = new GroupSequence(['Group1', 'Group2']); - $validatorMetadata = $this->createMock(ClassMetadataInterface::class); - $validatorMetadata->method('getGroupSequence')->willReturn($groupSequence); - - $metadata = new RequestDtoMetadata( - DummyRequestDto::class, - [], - $validatorMetadata, - ); - - $this->assertEquals(['Group1', 'Group2'], $metadata->getGroupSequence()); - } - - public function testGetGroupSequenceReturnsNullIfNoSequenceIsDefinedInValidatorMetadata(): void - { - $validatorMetadata = $this->createMock(ClassMetadataInterface::class); - $validatorMetadata->method('getGroupSequence')->willReturn(null); - - $metadata = new RequestDtoMetadata( - DummyRequestDto::class, - [], - $validatorMetadata, - ); - - $this->assertNull($metadata->getGroupSequence()); - } public function testNewInstancePassesArgumentsToDTOConstructorCorrectly(): void { @@ -135,54 +73,8 @@ public function testMetadataCanBeSerializedAndUnserializedPreservingAllPropertie $this->assertInstanceOf(RequestDtoMetadata::class, $unserialized); $this->assertEquals($metadata->getReflectionClass()->getName(), $unserialized->getReflectionClass()->getName()); $this->assertCount(2, iterator_to_array($unserialized->getPropertyMetadataGenerator())); - $this->assertTrue($unserialized->isConstrainedProperty('prop1')); - $this->assertEquals($validatorMetadata->getClassName(), $unserialized->getValidatorMetadata()->getClassName()); } - public function testGetAbstractParamAttributeFromPropertyReturnsNullIfNoAttributeIsFoundOnProperty(): void - { - $metadata = new RequestDtoMetadata( - DummyRequestDto::class, - [ - new RequestDtoParamMetadata(DummyRequestDto::class, 'prop1', 'mixed', true), - new RequestDtoParamMetadata(DummyRequestDto::class, 'prop2', 'mixed', false), - ], - $this->createMock(ClassMetadataInterface::class), - ); - - $property = $metadata->getPropertyMetadata('prop1')->getReflectionProperty(); - - $this->expectException(\LogicException::class); - $this->expectExceptionMessage('Property Crtl\RequestDtoResolverBundle\Test\Unit\Reflection\DummyRequestDto::$prop1 is missing an AbstractParam attribute.'); - - $metadata->getAbstractParamAttributeFromProperty($property); - } - - public function testGetAbstractParamAttributeFromPropertyTriggersWarningWhenMultipleAttributesAreFoundOnProperty(): void - { - $metadata = new RequestDtoMetadata( - DtoWithMultipleAttributes::class, - [ - new RequestDtoParamMetadata(DtoWithMultipleAttributes::class, 'prop', 'string', false), - ], - $this->createMock(ClassMetadataInterface::class), - ); - - $property = $metadata->getPropertyMetadata('prop')->getReflectionProperty(); - - set_error_handler(function ($errno, $errstr) { - $this->assertEquals(E_USER_WARNING, $errno); - $this->assertStringContainsString('Property Crtl\RequestDtoResolverBundle\Test\Unit\Reflection\DtoWithMultipleAttributes::$prop has more than one AbstractParam attribute', $errstr); - - return true; - }, E_USER_WARNING); - - $attribute = $metadata->getAbstractParamAttributeFromProperty($property); - - restore_error_handler(); - - $this->assertInstanceOf(QueryParam::class, $attribute); - } } final class DummyRequestDto diff --git a/test/Unit/Reflection/RequestDtoParamMetadataTest.php b/test/Unit/Reflection/RequestDtoParamMetadataTest.php index 87badd7..d53f604 100644 --- a/test/Unit/Reflection/RequestDtoParamMetadataTest.php +++ b/test/Unit/Reflection/RequestDtoParamMetadataTest.php @@ -156,6 +156,58 @@ public function testSerializeAndUnserializeWorksCorrectly(): void $this->assertEquals($metadata->isNestedDtoArray(), $unserialized->isNestedDtoArray()); $this->assertEquals($metadata->isNullable(), $unserialized->isNullable()); } + + public function testHasDefaultValueReturnsTrueWhenPropertyHasDefaultValue(): void + { + $metadata = new RequestDtoParamMetadata( + ParamMetadataDummyDto::class, + 'withDefaultValue', + 'string', + false, + null, + false, + false, + ); + + self::assertTrue($metadata->hasDefaultValue()); + + $metadata = new RequestDtoParamMetadata( + ParamMetadataDummyDto::class, + 'prop', + 'string', + false, + null, + false, + false, + ); + self::assertFalse($metadata->hasDefaultValue()); + } + + public function testGetDefaultValueReturnsDefaultValue(): void + { + $metadata = new RequestDtoParamMetadata( + ParamMetadataDummyDto::class, + 'withDefaultValue', + 'string', + false, + null, + false, + false, + ); + + self::assertSame("string", $metadata->getDefaultValue()); + + $metadata = new RequestDtoParamMetadata( + ParamMetadataDummyDto::class, + 'prop', + 'string', + false, + null, + false, + false, + ); + self::assertNull($metadata->getDefaultValue()); + } } final class ParamMetadataDummyDto @@ -168,6 +220,8 @@ final class ParamMetadataDummyDto #[QueryParam] #[BodyParam] public string $multipleAttributesProp; + + public string $withDefaultValue = "string"; } final class ParamMetadataNestedDto diff --git a/test/Unit/Utility/DtoReflectionHelperTest.php b/test/Unit/Utility/DtoReflectionHelperTest.php index 2623c62..242e9c0 100644 --- a/test/Unit/Utility/DtoReflectionHelperTest.php +++ b/test/Unit/Utility/DtoReflectionHelperTest.php @@ -41,6 +41,8 @@ public function testGetDtoClassNameFromReflectionProperty(): void #[BodyParam] public ?string $notDto; + + public $noType; }; $reflectionClass = new \ReflectionClass($class); @@ -53,6 +55,10 @@ public function testGetDtoClassNameFromReflectionProperty(): void $property = $reflectionClass->getProperty('notDto'); $type = $this->helper->getDtoClassNameFromReflectionProperty($property); $this->assertNull($type); + + $property = $reflectionClass->getProperty('noType'); + $type = $this->helper->getDtoClassNameFromReflectionProperty($property); + $this->assertNull($type); } public function testGetDtoClassNameFromReflectionPropertyThrowsRuntimeExceptionForUnionAndIntersectionTypes(): void @@ -149,4 +155,5 @@ public function testIsRequestDto(): void // @phpstan-ignore method.alreadyNarrowedType $this->assertFalse($this->helper->isRequestDto(\stdClass::class)); } + } From 4bac4c38ca115066b3f507a28354ea20a24cf279 Mon Sep 17 00:00:00 2001 From: Marvin Petker Date: Sun, 8 Feb 2026 06:19:42 +0100 Subject: [PATCH 4/6] refactor: remove unused stuff; optimizations --- config/services.php | 2 - src/Factory/RequestDtoFactory.php | 9 +- src/Reflection/RequestDtoMetadata.php | 31 ++-- src/Reflection/RequestDtoMetadataFactory.php | 10 +- src/Reflection/RequestDtoParamMetadata.php | 34 ++--- .../RequestDtoParamMetadataFactory.php | 13 +- src/Reflection/WithCacheTrait.php | 19 ++- src/Utility/TypeHelper.php | 5 + test/Unit/Attribute/BodyParamTest.php | 13 +- test/Unit/Attribute/QueryParamTest.php | 13 +- test/Unit/Attribute/RouteParamTest.php | 1 - .../RequestDtoMetadataFactoryTest.php | 35 +---- .../Reflection/RequestDtoMetadataTest.php | 20 +-- .../RequestDtoParamMetadataFactoryTest.php | 141 ++++++++++++++++++ .../RequestDtoParamMetadataTest.php | 45 ++---- test/Unit/Utility/DtoReflectionHelperTest.php | 2 +- 16 files changed, 227 insertions(+), 166 deletions(-) create mode 100644 test/Unit/Reflection/RequestDtoParamMetadataFactoryTest.php diff --git a/config/services.php b/config/services.php index ce67b0b..4811506 100644 --- a/config/services.php +++ b/config/services.php @@ -64,14 +64,12 @@ $services->set(RequestDtoParamMetadataFactory::class) ->args([ - '$validator' => service('validator'), '$reflectionHelper' => service(DtoReflectionHelper::class), '$propertyInfoExtractor' => service('crtl_request_dto_resolver_bundle.property_extractor'), ]); $services->set(RequestDtoMetadataFactory::class) ->args([ - '$validator' => service('validator'), '$reflectionHelper' => service(DtoReflectionHelper::class), '$requestDtoParamMetadataFactory' => service(RequestDtoParamMetadataFactory::class), '$cache' => service('cache.system'), diff --git a/src/Factory/RequestDtoFactory.php b/src/Factory/RequestDtoFactory.php index afaf3fd..b30e269 100644 --- a/src/Factory/RequestDtoFactory.php +++ b/src/Factory/RequestDtoFactory.php @@ -216,10 +216,9 @@ private function createInstanceRecursive( $resultArray = []; foreach ($valueArray as $i => $nestedValue) { - /** @var AbstractParam $nestedAttr */ - $nestedAttr = clone $attr; - if ($isArray && $nestedAttr instanceof AbstractNestedParam) { - $nestedAttr->setIndex($i); + if ($isArray && $attr instanceof AbstractNestedParam) { + $attr = clone $attr; + $attr->setIndex($i); } try { @@ -230,7 +229,7 @@ private function createInstanceRecursive( : $nestedValue, $valueProvider, $valueChecker, - $nestedAttr, + $attr, $nestedMetadata, $stack, ); diff --git a/src/Reflection/RequestDtoMetadata.php b/src/Reflection/RequestDtoMetadata.php index d0207cd..ace6931 100644 --- a/src/Reflection/RequestDtoMetadata.php +++ b/src/Reflection/RequestDtoMetadata.php @@ -13,8 +13,6 @@ namespace Crtl\RequestDtoResolverBundle\Reflection; -use Crtl\RequestDtoResolverBundle\Attribute\AbstractParam; -use Symfony\Component\Validator\Constraints\GroupSequence; use Symfony\Component\Validator\Mapping\ClassMetadataInterface; class RequestDtoMetadata @@ -24,6 +22,11 @@ class RequestDtoMetadata */ private array $propertyMetadata = []; + /** + * @var \ReflectionClass|null + */ + private ?\ReflectionClass $reflectionClass = null; + /** * @param RequestDtoParamMetadata[] $propertyMetadata */ @@ -35,19 +38,12 @@ public function __construct( */ private readonly string $className, - /** + /* * Property names mapped to metadata * * @var RequestDtoParamMetadata[] */ array $propertyMetadata, - - /** - * Validator metadata of the Request DTO. - * - * @var ClassMetadataInterface - */ - private readonly ClassMetadataInterface $validatorMetadata, ) { foreach ($propertyMetadata as $propMetadata) { $propertyName = $propMetadata->getPropertyName(); @@ -68,7 +64,6 @@ public function __serialize(): array return [ 'className' => $this->className, 'propertyMetadata' => $this->propertyMetadata, - 'validatorMetadata' => $this->validatorMetadata, ]; } @@ -84,14 +79,7 @@ public function __serialize(): array public function __unserialize(array $data): void { $this->className = $data['className']; - $this->validatorMetadata = $data['validatorMetadata']; $this->propertyMetadata = $data['propertyMetadata']; - - foreach ($this->propertyMetadata as $metadata) { - if ($metadata->isConstrained()) { - $this->constrainedProperties[$metadata->getPropertyName()] = $metadata; - } - } } /** @@ -106,10 +94,14 @@ public function getPropertyMetadata(string $propertyName): ?RequestDtoParamMetad * Returns reflection class of request dto. * * @return \ReflectionClass + * + * @throws \ReflectionException */ public function getReflectionClass(): \ReflectionClass { - return new \ReflectionClass($this->className); + $this->reflectionClass ??= new \ReflectionClass($this->className); + + return $this->reflectionClass; } public function getPropertyMetadataGenerator(): \Generator @@ -119,7 +111,6 @@ public function getPropertyMetadataGenerator(): \Generator } } - /** * @param mixed ...$args Arguments passed to new instance constructor, only if implemented * diff --git a/src/Reflection/RequestDtoMetadataFactory.php b/src/Reflection/RequestDtoMetadataFactory.php index c3444d2..6e2e584 100644 --- a/src/Reflection/RequestDtoMetadataFactory.php +++ b/src/Reflection/RequestDtoMetadataFactory.php @@ -16,15 +16,12 @@ use Crtl\RequestDtoResolverBundle\Utility\DtoReflectionHelper; use Psr\Cache\CacheItemPoolInterface; use Psr\Cache\InvalidArgumentException; -use Symfony\Component\Validator\Mapping\ClassMetadataInterface; -use Symfony\Component\Validator\Validator\ValidatorInterface; class RequestDtoMetadataFactory { use WithCacheTrait; public function __construct( - private readonly ValidatorInterface $validator, private readonly DtoReflectionHelper $reflectionHelper, private readonly RequestDtoParamMetadataFactory $requestDtoParamMetadataFactory, ?CacheItemPoolInterface $cache = null, @@ -34,7 +31,7 @@ public function __construct( /** * @param class-string $className - * @return RequestDtoMetadata + * * @throws InvalidArgumentException * @throws \ReflectionException */ @@ -51,10 +48,6 @@ public function getMetadataFor(string $className): RequestDtoMetadata throw new \LogicException(sprintf('DTO class "%s" must be instantiable.', $reflectionClass->getName())); } - $validatorMetadata = $this->validator->getMetadataFor($className); - - assert($validatorMetadata instanceof ClassMetadataInterface, 'Validator metadata for '.$className.' could not be retrieved'); - $properties = $this->reflectionHelper->getAttributedProperties($reflectionClass); $propertyMetadata = []; @@ -65,7 +58,6 @@ public function getMetadataFor(string $className): RequestDtoMetadata $metadata = new RequestDtoMetadata( $className, $propertyMetadata, - $validatorMetadata, ); $this->cacheValue($className, $metadata); diff --git a/src/Reflection/RequestDtoParamMetadata.php b/src/Reflection/RequestDtoParamMetadata.php index 611abd4..cf369ac 100644 --- a/src/Reflection/RequestDtoParamMetadata.php +++ b/src/Reflection/RequestDtoParamMetadata.php @@ -14,15 +14,14 @@ namespace Crtl\RequestDtoResolverBundle\Reflection; use Crtl\RequestDtoResolverBundle\Attribute\AbstractParam; -use Symfony\Component\TypeInfo\Type; class RequestDtoParamMetadata { /** - * @var \ReflectionClass + * @var \ReflectionClass|null */ - private \ReflectionClass $reflectionClass; - private \ReflectionProperty $reflectionProperty; + private ?\ReflectionClass $reflectionClass = null; + private ?\ReflectionProperty $reflectionProperty = null; public function __construct( /** @@ -59,13 +58,6 @@ public function __construct( */ private readonly string $builtInType, - /** - * Whether the property is constrained by validation. - * - * @var bool - */ - private readonly bool $isConstrained, - /** * Typehint classname of nested dto class or null if not a dto. * @@ -88,11 +80,6 @@ public function getBuiltinType(): string return $this->builtInType; } - public function isConstrained(): bool - { - return $this->isConstrained; - } - public function isNestedDtoArray(): bool { return $this->isNestedDtoArray; @@ -139,24 +126,25 @@ public function getDefaultValue(): mixed */ public function getReflectionClass(): \ReflectionClass { - if (!isset($this->reflectionClass)) { - $this->reflectionClass = new \ReflectionClass($this->className); - } + $this->reflectionClass ??= new \ReflectionClass($this->className); return $this->reflectionClass; } + /** + * @throws \ReflectionException + */ public function getReflectionProperty(): \ReflectionProperty { - if (!isset($this->reflectionProperty)) { - $this->reflectionProperty = $this->getReflectionClass()->getProperty($this->propertyName); - } + $this->reflectionProperty ??= $this->getReflectionClass()->getProperty($this->propertyName); return $this->reflectionProperty; } /** * Returns the AbstractParam attribute for this property, optionally with a parent. + * + * @throws \ReflectionException */ public function getAttribute(?AbstractParam $parent = null): AbstractParam { @@ -196,7 +184,6 @@ public function __serialize(): array 'className' => $this->className, 'propertyName' => $this->propertyName, 'builtInType' => $this->builtInType, - 'isConstrained' => $this->isConstrained, 'nestedDtoClassName' => $this->nestedDtoClassName, 'isNestedDtoArray' => $this->isNestedDtoArray, 'isNullable' => $this->isNullable, @@ -219,7 +206,6 @@ public function __unserialize(array $data): void $this->className = $data['className']; $this->propertyName = $data['propertyName']; $this->builtInType = $data['builtInType']; - $this->isConstrained = $data['isConstrained']; $this->nestedDtoClassName = $data['nestedDtoClassName']; $this->isNestedDtoArray = $data['isNestedDtoArray']; $this->isNullable = $data['isNullable']; diff --git a/src/Reflection/RequestDtoParamMetadataFactory.php b/src/Reflection/RequestDtoParamMetadataFactory.php index 6e3ef2d..d8f9fa1 100644 --- a/src/Reflection/RequestDtoParamMetadataFactory.php +++ b/src/Reflection/RequestDtoParamMetadataFactory.php @@ -20,13 +20,10 @@ use Symfony\Component\TypeInfo\Type\CollectionType; use Symfony\Component\TypeInfo\Type\ObjectType; use Symfony\Component\TypeInfo\Type\UnionType; -use Symfony\Component\Validator\Mapping\ClassMetadataInterface; -use Symfony\Component\Validator\Validator\ValidatorInterface; class RequestDtoParamMetadataFactory { public function __construct( - private readonly ValidatorInterface $validator, private readonly DtoReflectionHelper $reflectionHelper, private readonly PropertyInfoExtractorInterface $propertyInfoExtractor, ) { @@ -35,16 +32,12 @@ public function __construct( public function getMetadataFor(\ReflectionProperty $property): RequestDtoParamMetadata { $className = $property->getDeclaringClass()->getName(); - - /** @var ClassMetadataInterface $validatorClassMetadata */ - $validatorClassMetadata = $this->validator->getMetadataFor($className); - $constrainedProperties = $validatorClassMetadata->getConstrainedProperties(); - $propertyName = $property->getName(); try { $type = $this->propertyInfoExtractor->getType($className, $propertyName); } catch (\InvalidArgumentException $e) { + // @codeCoverageIgnoreStart // Compatibility fix because somehow type-info does not support unions with mixed. if ('Cannot create union with "mixed" standalone type.' !== $e->getMessage()) { throw $e; @@ -57,6 +50,7 @@ public function getMetadataFor(\ReflectionProperty $property): RequestDtoParamMe // Defensive fallback for invalid PHPDoc unions involving `mixed` $type = Type::mixed(); + // @codeCoverageIgnoreEnd } $typeDescription = TypeHelper::describe($type); @@ -91,8 +85,7 @@ public function getMetadataFor(\ReflectionProperty $property): RequestDtoParamMe $metadata = new RequestDtoParamMetadata( $property->getDeclaringClass()->getName(), $propertyName, - $typeDescription['builtInType'], - in_array($propertyName, $constrainedProperties, true), + strtolower($typeDescription['builtInType']), $definetlyClassString, $isArrayType, // check if type is nullable and fall back to true when no type specified diff --git a/src/Reflection/WithCacheTrait.php b/src/Reflection/WithCacheTrait.php index 06e768f..950780e 100644 --- a/src/Reflection/WithCacheTrait.php +++ b/src/Reflection/WithCacheTrait.php @@ -15,6 +15,7 @@ use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; +use Psr\Cache\InvalidArgumentException; /** * Traits to help implement caching. @@ -43,8 +44,6 @@ public function getCacheKey(string $className): string /** * @param class-string $className - * - * @throws \Psr\Cache\InvalidArgumentException */ private function getCacheItem(string $className): ?CacheItemInterface { @@ -53,7 +52,11 @@ private function getCacheItem(string $className): ?CacheItemInterface } if (!array_key_exists($className, $this->cacheItems)) { $cacheKey = $this->getCacheKey($className); - $cacheItem = $this->cache->getItem($cacheKey); + try { + $cacheItem = $this->cache->getItem($cacheKey); + } catch (InvalidArgumentException) { + $cacheItem = null; + } $this->cacheItems[$className] = $cacheItem; } @@ -62,12 +65,14 @@ private function getCacheItem(string $className): ?CacheItemInterface /** * @param class-string $className - * - * @throws \Psr\Cache\InvalidArgumentException */ private function getCachedValue(string $className): ?object { - $cacheItem = $this->getCacheItem($className); + try { + $cacheItem = $this->getCacheItem($className); + } catch (InvalidArgumentException) { + return null; + } if ($cacheItem?->isHit()) { return $cacheItem->get(); @@ -79,7 +84,7 @@ private function getCachedValue(string $className): ?object /** * @param class-string $className * - * @throws \Psr\Cache\InvalidArgumentException + * @throws InvalidArgumentException */ private function cacheValue(string $className, object $metadata): void { diff --git a/src/Utility/TypeHelper.php b/src/Utility/TypeHelper.php index ee8cdf9..347cd9c 100644 --- a/src/Utility/TypeHelper.php +++ b/src/Utility/TypeHelper.php @@ -18,6 +18,11 @@ use Symfony\Component\TypeInfo\Type\NullableType; use Symfony\Component\TypeInfo\Type\UnionType; +/** + * Internal helper used to process type info {@link Type}. + * + * @codeCoverageIgnore + */ final class TypeHelper { /** diff --git a/test/Unit/Attribute/BodyParamTest.php b/test/Unit/Attribute/BodyParamTest.php index 3684cf8..07b69d8 100644 --- a/test/Unit/Attribute/BodyParamTest.php +++ b/test/Unit/Attribute/BodyParamTest.php @@ -14,7 +14,6 @@ namespace Crtl\RequestDtoResolverBundle\Test\Unit\Attribute; use Crtl\RequestDtoResolverBundle\Attribute\BodyParam; -use Crtl\RequestDtoResolverBundle\Attribute\QueryParam; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; @@ -72,9 +71,9 @@ public function testReadsValueFromJsonRequest(): void public function testHasValueInRequestReturnsWhetherValueIsExistsInRequest(): void { - $request = new Request(request: ["param" => "value"]); + $request = new Request(request: ['param' => 'value']); - $param = new BodyParam("param"); + $param = new BodyParam('param'); $this->assertTrue($param->hasValueInRequest($request)); $this->assertFalse($param->hasValueInRequest(new Request())); @@ -85,8 +84,8 @@ public function testHasValueInRequestWithNestedParam(): void $parent = new BodyParam('parent'); $child = new BodyParam('child'); $request = new Request(request: [ - "parent" => [ - "child" => "value", + 'parent' => [ + 'child' => 'value', ] ]); @@ -101,8 +100,8 @@ public function testHasValueInRequestWithNestedArrayParam(): void $parent = new BodyParam('parent'); $child = new BodyParam('child'); $request = new Request(request: [ - "parent" => [ - ["child" => "value",] + 'parent' => [ + ['child' => 'value'] ] ]); diff --git a/test/Unit/Attribute/QueryParamTest.php b/test/Unit/Attribute/QueryParamTest.php index e8a2101..fd6816b 100644 --- a/test/Unit/Attribute/QueryParamTest.php +++ b/test/Unit/Attribute/QueryParamTest.php @@ -13,7 +13,6 @@ namespace Crtl\RequestDtoResolverBundle\Test\Unit\Attribute; -use Crtl\RequestDtoResolverBundle\Attribute\HeaderParam; use Crtl\RequestDtoResolverBundle\Attribute\QueryParam; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -104,9 +103,9 @@ public function testGetValueFromRequestReturnsArrayWithoutTransformationIfValueI public function testHasValueInRequestReturnsWhetherValueIsExistsInRequest(): void { - $request = new Request(["param" => "value"], [], [], [], [], []); + $request = new Request(['param' => 'value'], [], [], [], [], []); - $param = new QueryParam("param"); + $param = new QueryParam('param'); $this->assertTrue($param->hasValueInRequest($request)); $this->assertFalse($param->hasValueInRequest(new Request())); @@ -117,8 +116,8 @@ public function testHasValueInRequestWithNestedParam(): void $parent = new QueryParam('parent'); $child = new QueryParam('child'); $request = new Request([ - "parent" => [ - "child" => "value", + 'parent' => [ + 'child' => 'value', ] ]); @@ -133,8 +132,8 @@ public function testHasValueInRequestWithNestedArrayParam(): void $parent = new QueryParam('parent'); $child = new QueryParam('child'); $request = new Request([ - "parent" => [ - ["child" => "value",] + 'parent' => [ + ['child' => 'value'] ] ]); diff --git a/test/Unit/Attribute/RouteParamTest.php b/test/Unit/Attribute/RouteParamTest.php index dfd6bcd..0740099 100644 --- a/test/Unit/Attribute/RouteParamTest.php +++ b/test/Unit/Attribute/RouteParamTest.php @@ -13,7 +13,6 @@ namespace Crtl\RequestDtoResolverBundle\Test\Unit\Attribute; -use Crtl\RequestDtoResolverBundle\Attribute\HeaderParam; use Crtl\RequestDtoResolverBundle\Attribute\RouteParam; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; diff --git a/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php b/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php index 5f91c4f..2e4323c 100644 --- a/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php +++ b/test/Unit/Reflection/RequestDtoMetadataFactoryTest.php @@ -23,12 +23,9 @@ use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; use Symfony\Component\Validator\Mapping\ClassMetadataInterface; -use Symfony\Component\Validator\Validator\ValidatorInterface; final class RequestDtoMetadataFactoryTest extends TestCase { - private ValidatorInterface&MockObject $validator; - private DtoReflectionHelper&MockObject $reflectionHelper; private RequestDtoParamMetadataFactory&MockObject $paramMetadataFactory; @@ -37,10 +34,9 @@ final class RequestDtoMetadataFactoryTest extends TestCase protected function setUp(): void { - $this->validator = $this->createMock(ValidatorInterface::class); $this->reflectionHelper = $this->createMock(DtoReflectionHelper::class); $this->paramMetadataFactory = $this->createMock(RequestDtoParamMetadataFactory::class); - $this->factory = new RequestDtoMetadataFactory($this->validator, $this->reflectionHelper, $this->paramMetadataFactory); + $this->factory = new RequestDtoMetadataFactory($this->reflectionHelper, $this->paramMetadataFactory); } public function testGetMetadataForReturnsRequestDtoMetadataForGivenClassName(): void @@ -48,11 +44,6 @@ public function testGetMetadataForReturnsRequestDtoMetadataForGivenClassName(): $className = DummyDto::class; $validatorMetadata = $this->createMock(ClassMetadataInterface::class); - $this->validator->expects($this->once()) - ->method('getMetadataFor') - ->with($className) - ->willReturn($validatorMetadata); - $prop1 = new \ReflectionProperty($className, 'prop1'); $prop2 = new \ReflectionProperty($className, 'prop2'); @@ -63,7 +54,7 @@ public function testGetMetadataForReturnsRequestDtoMetadataForGivenClassName(): $this->paramMetadataFactory->expects($this->exactly(2)) ->method('getMetadataFor') ->willReturnCallback(function (\ReflectionProperty $prop) { - return new RequestDtoParamMetadata($prop->getDeclaringClass()->getName(), $prop->getName(), 'mixed', 'prop1' === $prop->getName()); + return new RequestDtoParamMetadata($prop->getDeclaringClass()->getName(), $prop->getName(), 'mixed'); }); $metadata = $this->factory->getMetadataFor($className); @@ -78,7 +69,7 @@ public function testGetMetadataForReturnsCachedMetadataOnCacheHit(): void $cacheItem = $this->createMock(CacheItemInterface::class); $cachedMetadata = $this->createMock(RequestDtoMetadata::class); - $factory = new RequestDtoMetadataFactory($this->validator, $this->reflectionHelper, $this->paramMetadataFactory, $cache); + $factory = new RequestDtoMetadataFactory($this->reflectionHelper, $this->paramMetadataFactory, $cache); $className = DummyDto::class; $cacheKey = $factory->getCacheKey($className); // str_replace('\\', '_', $className).'_'.str_replace('\\', '_', RequestDtoMetadata::class); @@ -96,8 +87,6 @@ public function testGetMetadataForReturnsCachedMetadataOnCacheHit(): void ->method('get') ->willReturn($cachedMetadata); - $this->validator->expects($this->never())->method('getMetadataFor'); - $metadata = $factory->getMetadataFor($className); $this->assertSame($cachedMetadata, $metadata); @@ -108,7 +97,7 @@ public function testGetMetadataForCreatesAndCachesMetadataOnCacheMiss(): void $cache = $this->createMock(CacheItemPoolInterface::class); $cacheItem = $this->createMock(CacheItemInterface::class); - $factory = new RequestDtoMetadataFactory($this->validator, $this->reflectionHelper, $this->paramMetadataFactory, $cache); + $factory = new RequestDtoMetadataFactory($this->reflectionHelper, $this->paramMetadataFactory, $cache); $className = DummyDto::class; $cacheKey = $factory->getCacheKey($className); // str_replace('\\', '_', $className).'_'.str_replace('\\', '_', RequestDtoMetadata::class); @@ -122,13 +111,6 @@ public function testGetMetadataForCreatesAndCachesMetadataOnCacheMiss(): void ->method('isHit') ->willReturn(false); - $validatorMetadata = $this->createMock(ClassMetadataInterface::class); - - $this->validator->expects($this->once()) - ->method('getMetadataFor') - ->with($className) - ->willReturn($validatorMetadata); - $this->reflectionHelper->expects($this->once()) ->method('getAttributedProperties') ->willReturn([]); @@ -145,9 +127,6 @@ public function testGetMetadataForCreatesAndCachesMetadataOnCacheMiss(): void public function testGetMetadataForThrowsRuntimeExceptionWhenUnsupportedTypeIsEncountered(): void { $className = DummyDto::class; - $validatorMetadata = $this->createMock(ClassMetadataInterface::class); - - $this->validator->method('getMetadataFor')->willReturn($validatorMetadata); $prop1 = new \ReflectionProperty($className, 'prop1'); @@ -182,9 +161,9 @@ final class DummyDto public string $prop2; } -final class PrivateConstructor { +final class PrivateConstructor +{ private function __construct() { - } -} \ No newline at end of file +} diff --git a/test/Unit/Reflection/RequestDtoMetadataTest.php b/test/Unit/Reflection/RequestDtoMetadataTest.php index 6e1bde5..1ceaf94 100644 --- a/test/Unit/Reflection/RequestDtoMetadataTest.php +++ b/test/Unit/Reflection/RequestDtoMetadataTest.php @@ -18,23 +18,17 @@ use Crtl\RequestDtoResolverBundle\Reflection\RequestDtoMetadata; use Crtl\RequestDtoResolverBundle\Reflection\RequestDtoParamMetadata; use PHPUnit\Framework\TestCase; -use Symfony\Component\Validator\Constraints\GroupSequence; -use Symfony\Component\Validator\Mapping\ClassMetadata; -use Symfony\Component\Validator\Mapping\ClassMetadataInterface; final class RequestDtoMetadataTest extends TestCase { public function testMetadataAccessorsReturnCorrectValues(): void { - $validatorMetadata = $this->createMock(ClassMetadataInterface::class); - $metadata = new RequestDtoMetadata( DummyRequestDto::class, [ - new RequestDtoParamMetadata(DummyRequestDto::class, 'prop1', 'string', false), - new RequestDtoParamMetadata(DummyRequestDto::class, 'prop2', 'string', false), + new RequestDtoParamMetadata(DummyRequestDto::class, 'prop1', 'string'), + new RequestDtoParamMetadata(DummyRequestDto::class, 'prop2', 'string'), ], - $validatorMetadata, ); $this->assertCount(2, iterator_to_array($metadata->getPropertyMetadataGenerator())); @@ -42,10 +36,9 @@ public function testMetadataAccessorsReturnCorrectValues(): void $this->assertEquals(DummyRequestDto::class, $metadata->getReflectionClass()->getName()); } - public function testNewInstancePassesArgumentsToDTOConstructorCorrectly(): void { - $metadata = new RequestDtoMetadata(RequestDtoWithConstructor::class, [], $this->createMock(ClassMetadataInterface::class)); + $metadata = new RequestDtoMetadata(RequestDtoWithConstructor::class, []); $instance = $metadata->newInstance('test-param', 1, 2); $this->assertInstanceOf(RequestDtoWithConstructor::class, $instance); @@ -54,14 +47,12 @@ public function testNewInstancePassesArgumentsToDTOConstructorCorrectly(): void public function testMetadataCanBeSerializedAndUnserializedPreservingAllProperties(): void { - $validatorMetadata = new ClassMetadata(DummyRequestDto::class); $metadata = new RequestDtoMetadata( DummyRequestDto::class, [ - new RequestDtoParamMetadata(DummyRequestDto::class, 'prop1', 'mixed', true), - new RequestDtoParamMetadata(DummyRequestDto::class, 'prop2', 'mixed', false), + new RequestDtoParamMetadata(DummyRequestDto::class, 'prop1', 'mixed'), + new RequestDtoParamMetadata(DummyRequestDto::class, 'prop2', 'mixed'), ], - $validatorMetadata, ); // Access properties to populate internal state if any @@ -74,7 +65,6 @@ public function testMetadataCanBeSerializedAndUnserializedPreservingAllPropertie $this->assertEquals($metadata->getReflectionClass()->getName(), $unserialized->getReflectionClass()->getName()); $this->assertCount(2, iterator_to_array($unserialized->getPropertyMetadataGenerator())); } - } final class DummyRequestDto diff --git a/test/Unit/Reflection/RequestDtoParamMetadataFactoryTest.php b/test/Unit/Reflection/RequestDtoParamMetadataFactoryTest.php new file mode 100644 index 0000000..5671dbf --- /dev/null +++ b/test/Unit/Reflection/RequestDtoParamMetadataFactoryTest.php @@ -0,0 +1,141 @@ +reflectionHelper = $this->createMock(DtoReflectionHelper::class); + $this->propertyInfoExtractor = $this->createMock(TestExtractorInterface::class); + $this->factory = new RequestDtoParamMetadataFactory( + $this->reflectionHelper, + $this->propertyInfoExtractor, + ); + } + + public function testGetMetadataForBuildsMetadataForConstrainedProperty(): void + { + $className = ParamMetadataFactoryDummyDto::class; + $property = new \ReflectionProperty($className, 'constrainedProp'); + + $this->propertyInfoExtractor->expects($this->once()) + ->method('getType') + ->with($className, 'constrainedProp') + ->willReturn(Type::string()); + + $this->reflectionHelper->expects($this->never()) + ->method('isRequestDto'); + + $metadata = $this->factory->getMetadataFor($property); + + $this->assertSame('string', $metadata->getBuiltinType()); + $this->assertFalse($metadata->isNestedDtoArray()); + $this->assertNull($metadata->getNestedDtoClassName()); + $this->assertFalse($metadata->isNullable()); + } + + public function testGetMetadataForDetectsNestedDtoInArrayType(): void + { + $className = ParamMetadataFactoryDummyDto::class; + $property = new \ReflectionProperty($className, 'nestedArray'); + + $this->propertyInfoExtractor->expects($this->once()) + ->method('getType') + ->with($className, 'nestedArray') + ->willReturn(Type::array(Type::object(ParamMetadataFactoryNestedDto::class))); + + $this->reflectionHelper->expects($this->once()) + ->method('isRequestDto') + ->with(ParamMetadataFactoryNestedDto::class) + ->willReturn(true); + + $metadata = $this->factory->getMetadataFor($property); + + $this->assertSame('array', $metadata->getBuiltinType()); + $this->assertTrue($metadata->isNestedDtoArray()); + $this->assertSame(ParamMetadataFactoryNestedDto::class, $metadata->getNestedDtoClassName()); + $this->assertFalse($metadata->isNullable()); + } + + public function testGetMetadataForTriggersWarningOnMixedUnionFallback(): void + { + $className = ParamMetadataFactoryDummyDto::class; + $property = new \ReflectionProperty($className, 'mixedUnion'); + + $this->propertyInfoExtractor->expects($this->once()) + ->method('getType') + ->with($className, 'mixedUnion') + ->willThrowException(new \InvalidArgumentException('Cannot create union with "mixed" standalone type.')); + + $warningTriggered = false; + set_error_handler(function ($errno, $errstr) use (&$warningTriggered) { + if (E_USER_WARNING === $errno && str_contains($errstr, 'Unable to guess type for mixed union type')) { + $warningTriggered = true; + + return true; + } + + return false; + }); + + $metadata = $this->factory->getMetadataFor($property); + restore_error_handler(); + + $this->assertTrue($warningTriggered); + $this->assertSame('mixed', $metadata->getBuiltinType()); + $this->assertFalse($metadata->isNestedDtoArray()); + $this->assertNull($metadata->getNestedDtoClassName()); + $this->assertTrue($metadata->isNullable()); + } +} + +final class ParamMetadataFactoryDummyDto +{ + public string $constrainedProp; + + /** + * @var mixed[] + */ + public array $nestedArray; + + // @phpstan-ignore missingType.property + public $mixedUnion; +} + +final class ParamMetadataFactoryNestedDto +{ +} diff --git a/test/Unit/Reflection/RequestDtoParamMetadataTest.php b/test/Unit/Reflection/RequestDtoParamMetadataTest.php index d53f604..c3e2c60 100644 --- a/test/Unit/Reflection/RequestDtoParamMetadataTest.php +++ b/test/Unit/Reflection/RequestDtoParamMetadataTest.php @@ -23,44 +23,35 @@ final class RequestDtoParamMetadataTest extends TestCase { public function testGetClassNameReturnsCorrectClassName(): void { - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', false); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed'); $this->assertEquals(ParamMetadataDummyDto::class, $metadata->getClassName()); } public function testGetPropertyNameReturnsCorrectPropertyName(): void { - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', false); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed'); $this->assertEquals('prop', $metadata->getPropertyName()); } - public function testIsConstrainedReturnsCorrectValue(): void - { - $metadataTrue = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'string', true); - $this->assertTrue($metadataTrue->isConstrained()); - - $metadataFalse = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'string', false); - $this->assertFalse($metadataFalse->isConstrained()); - } - public function testGetNestedDtoClassNameReturnsCorrectClassName(): void { - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', true, ParamMetadataNestedDto::class); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', ParamMetadataNestedDto::class); $this->assertEquals(ParamMetadataNestedDto::class, $metadata->getNestedDtoClassName()); - $metadataNull = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', true, null); + $metadataNull = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', null); $this->assertNull($metadataNull->getNestedDtoClassName()); } public function testGetReflectionClassReturnsCorrectReflectionClass(): void { - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', false); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed'); $reflection = $metadata->getReflectionClass(); $this->assertEquals(ParamMetadataDummyDto::class, $reflection->getName()); } public function testGetReflectionPropertyReturnsCorrectReflectionProperty(): void { - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', true); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed'); $reflection = $metadata->getReflectionProperty(); $this->assertEquals('prop', $reflection->getName()); $this->assertEquals(ParamMetadataDummyDto::class, $reflection->getDeclaringClass()->getName()); @@ -68,7 +59,7 @@ public function testGetReflectionPropertyReturnsCorrectReflectionProperty(): voi public function testGetAttributeReturnsAbstractParamAttribute(): void { - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', true); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed'); $attribute = $metadata->getAttribute(); $this->assertInstanceOf(QueryParam::class, $attribute); $this->assertEquals('prop', $attribute->getName()); @@ -76,7 +67,7 @@ public function testGetAttributeReturnsAbstractParamAttribute(): void public function testGetAttributeSetsParentAttributeWhenProvided(): void { - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', true); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed'); $parent = new BodyParam(); $attribute = $metadata->getAttribute($parent); @@ -87,7 +78,7 @@ public function testGetAttributeSetsParentAttributeWhenProvided(): void public function testGetAttributeThrowsLogicExceptionWhenAttributeIsMissing(): void { - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'noAttributeProp', 'mixed', true); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'noAttributeProp', 'mixed'); $this->expectException(\LogicException::class); $this->expectExceptionMessage('Property Crtl\RequestDtoResolverBundle\Test\Unit\Reflection\ParamMetadataDummyDto::$noAttributeProp is missing an AbstractParam attribute.'); $metadata->getAttribute(); @@ -95,7 +86,7 @@ public function testGetAttributeThrowsLogicExceptionWhenAttributeIsMissing(): vo public function testGetAttributeTriggersWarningWhenMultipleAttributesArePresent(): void { - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'multipleAttributesProp', 'mixed', true); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'multipleAttributesProp', 'mixed'); $warningTriggered = false; set_error_handler(function ($errno, $errstr) use (&$warningTriggered) { @@ -117,7 +108,7 @@ public function testGetAttributeTriggersWarningWhenMultipleAttributesArePresent( public function testSetValueSetsPropertyValueOnDto(): void { - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', true); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed'); $dto = new ParamMetadataDummyDto(); $metadata->setValue($dto, 'new value'); $this->assertEquals('new value', $dto->prop); @@ -125,10 +116,10 @@ public function testSetValueSetsPropertyValueOnDto(): void public function testIsNullable(): void { - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', true); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed'); self::assertFalse($metadata->isNullable()); - $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', true, isNullable: true); + $metadata = new RequestDtoParamMetadata(ParamMetadataDummyDto::class, 'prop', 'mixed', isNullable: true); self::assertTrue($metadata->isNullable()); } @@ -138,7 +129,6 @@ public function testSerializeAndUnserializeWorksCorrectly(): void ParamMetadataDummyDto::class, 'prop', 'string', - true, ParamMetadataNestedDto::class, true, true, @@ -151,7 +141,6 @@ public function testSerializeAndUnserializeWorksCorrectly(): void $this->assertEquals($metadata->getClassName(), $unserialized->getClassName()); $this->assertEquals($metadata->getPropertyName(), $unserialized->getPropertyName()); $this->assertEquals($metadata->getBuiltinType(), $unserialized->getBuiltinType()); - $this->assertEquals($metadata->isConstrained(), $unserialized->isConstrained()); $this->assertEquals($metadata->getNestedDtoClassName(), $unserialized->getNestedDtoClassName()); $this->assertEquals($metadata->isNestedDtoArray(), $unserialized->isNestedDtoArray()); $this->assertEquals($metadata->isNullable(), $unserialized->isNullable()); @@ -163,7 +152,6 @@ public function testHasDefaultValueReturnsTrueWhenPropertyHasDefaultValue(): voi ParamMetadataDummyDto::class, 'withDefaultValue', 'string', - false, null, false, false, @@ -175,7 +163,6 @@ public function testHasDefaultValueReturnsTrueWhenPropertyHasDefaultValue(): voi ParamMetadataDummyDto::class, 'prop', 'string', - false, null, false, false, @@ -189,19 +176,17 @@ public function testGetDefaultValueReturnsDefaultValue(): void ParamMetadataDummyDto::class, 'withDefaultValue', 'string', - false, null, false, false, ); - self::assertSame("string", $metadata->getDefaultValue()); + self::assertSame('string', $metadata->getDefaultValue()); $metadata = new RequestDtoParamMetadata( ParamMetadataDummyDto::class, 'prop', 'string', - false, null, false, false, @@ -221,7 +206,7 @@ final class ParamMetadataDummyDto #[BodyParam] public string $multipleAttributesProp; - public string $withDefaultValue = "string"; + public string $withDefaultValue = 'string'; } final class ParamMetadataNestedDto diff --git a/test/Unit/Utility/DtoReflectionHelperTest.php b/test/Unit/Utility/DtoReflectionHelperTest.php index 242e9c0..f876b00 100644 --- a/test/Unit/Utility/DtoReflectionHelperTest.php +++ b/test/Unit/Utility/DtoReflectionHelperTest.php @@ -42,6 +42,7 @@ public function testGetDtoClassNameFromReflectionProperty(): void #[BodyParam] public ?string $notDto; + // @phpstan-ignore missingType.property public $noType; }; @@ -155,5 +156,4 @@ public function testIsRequestDto(): void // @phpstan-ignore method.alreadyNarrowedType $this->assertFalse($this->helper->isRequestDto(\stdClass::class)); } - } From 34778b9d92c305c305c116185f18e38422b921b3 Mon Sep 17 00:00:00 2001 From: Marvin Petker Date: Sun, 8 Feb 2026 17:22:25 +0100 Subject: [PATCH 5/6] feat: Add strict flag to RequestDto to allow controlling type coercion when properties are assigned. --- src/Attribute/RequestDto.php | 10 +++++ src/Factory/RequestDtoFactory.php | 11 ++--- src/Reflection/RequestDtoMetadata.php | 26 +++++++++++ src/Reflection/RequestDtoMetadataFactory.php | 2 - src/Reflection/WithCacheTrait.php | 15 +------ test/Fixtures/NonStrictTypeConflictingDto.php | 39 ++++++++++++++++ test/Fixtures/TypeConflictingDto.php | 2 +- ...equestDtoResolverBundleIntegrationTest.php | 44 +++++++++++++++++++ test/Unit/Attribute/RequestDtoTest.php | 41 +++++++++++++++++ 9 files changed, 169 insertions(+), 21 deletions(-) create mode 100644 test/Fixtures/NonStrictTypeConflictingDto.php create mode 100644 test/Unit/Attribute/RequestDtoTest.php diff --git a/src/Attribute/RequestDto.php b/src/Attribute/RequestDto.php index 1e44707..37dec8e 100644 --- a/src/Attribute/RequestDto.php +++ b/src/Attribute/RequestDto.php @@ -22,4 +22,14 @@ #[\Attribute(\Attribute::TARGET_CLASS)] class RequestDto { + public function __construct( + /** + * Whether the DTO should be hydrated in strict mode. + * + * By defaults properties may cause type constraint violations when the required types mismatches with input. + * If you want to attempt to coerce values instead pass `false` instead. + */ + public readonly bool $strict = true, + ) { + } } diff --git a/src/Factory/RequestDtoFactory.php b/src/Factory/RequestDtoFactory.php index b30e269..e875344 100644 --- a/src/Factory/RequestDtoFactory.php +++ b/src/Factory/RequestDtoFactory.php @@ -262,7 +262,7 @@ private function createInstanceRecursive( } try { - $this->assignProperty($object, $propertyMetadata, $value); + $this->assignProperty($object, $metadata, $propertyMetadata, $value); } catch (PropertyHydrationException $e) { $violation = $this->createTypeErrorViolation( $e->typeError, @@ -288,14 +288,15 @@ private function createInstanceRecursive( */ private function assignProperty( object $object, - RequestDtoParamMetadata $metadata, + RequestDtoMetadata $classMetadata, + RequestDtoParamMetadata $propertyMetadata, mixed $value, ): void { try { - $propertyName = $metadata->getPropertyName(); - $object->$propertyName = $value; + $propertyName = $propertyMetadata->getPropertyName(); + $classMetadata->assignPropertyValue($object, $propertyName, $value); } catch (\TypeError $e) { - throw new PropertyHydrationException(get_class($object), $metadata->getPropertyName(), $value, $e); + throw new PropertyHydrationException(get_class($object), $propertyMetadata->getPropertyName(), $value, $e); } } diff --git a/src/Reflection/RequestDtoMetadata.php b/src/Reflection/RequestDtoMetadata.php index ace6931..e15f11e 100644 --- a/src/Reflection/RequestDtoMetadata.php +++ b/src/Reflection/RequestDtoMetadata.php @@ -13,6 +13,7 @@ namespace Crtl\RequestDtoResolverBundle\Reflection; +use Crtl\RequestDtoResolverBundle\Attribute\RequestDto; use Symfony\Component\Validator\Mapping\ClassMetadataInterface; class RequestDtoMetadata @@ -111,6 +112,31 @@ public function getPropertyMetadataGenerator(): \Generator } } + /** + * @throws \ReflectionException + * @throws \TypeError + */ + public function assignPropertyValue(object $object, string $property, mixed $value): void + { + $reflectionClass = new \ReflectionClass($object); + $attrs = $reflectionClass->getAttributes(RequestDto::class, \ReflectionAttribute::IS_INSTANCEOF); + + $strict = false; + + if (count($attrs) > 0) { + /** @var RequestDto $attr */ + $attr = $attrs[0]->newInstance(); + $strict = $attr->strict; + } + + if ($strict) { + $object->$property = $value; + } else { + $reflectionClass->getProperty($property) + ->setValue($object, $value); + } + } + /** * @param mixed ...$args Arguments passed to new instance constructor, only if implemented * diff --git a/src/Reflection/RequestDtoMetadataFactory.php b/src/Reflection/RequestDtoMetadataFactory.php index 6e2e584..041fd42 100644 --- a/src/Reflection/RequestDtoMetadataFactory.php +++ b/src/Reflection/RequestDtoMetadataFactory.php @@ -15,7 +15,6 @@ use Crtl\RequestDtoResolverBundle\Utility\DtoReflectionHelper; use Psr\Cache\CacheItemPoolInterface; -use Psr\Cache\InvalidArgumentException; class RequestDtoMetadataFactory { @@ -32,7 +31,6 @@ public function __construct( /** * @param class-string $className * - * @throws InvalidArgumentException * @throws \ReflectionException */ public function getMetadataFor(string $className): RequestDtoMetadata diff --git a/src/Reflection/WithCacheTrait.php b/src/Reflection/WithCacheTrait.php index 950780e..293ec96 100644 --- a/src/Reflection/WithCacheTrait.php +++ b/src/Reflection/WithCacheTrait.php @@ -15,7 +15,6 @@ use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; -use Psr\Cache\InvalidArgumentException; /** * Traits to help implement caching. @@ -52,11 +51,7 @@ private function getCacheItem(string $className): ?CacheItemInterface } if (!array_key_exists($className, $this->cacheItems)) { $cacheKey = $this->getCacheKey($className); - try { - $cacheItem = $this->cache->getItem($cacheKey); - } catch (InvalidArgumentException) { - $cacheItem = null; - } + $cacheItem = $this->cache->getItem($cacheKey); $this->cacheItems[$className] = $cacheItem; } @@ -68,11 +63,7 @@ private function getCacheItem(string $className): ?CacheItemInterface */ private function getCachedValue(string $className): ?object { - try { - $cacheItem = $this->getCacheItem($className); - } catch (InvalidArgumentException) { - return null; - } + $cacheItem = $this->getCacheItem($className); if ($cacheItem?->isHit()) { return $cacheItem->get(); @@ -83,8 +74,6 @@ private function getCachedValue(string $className): ?object /** * @param class-string $className - * - * @throws InvalidArgumentException */ private function cacheValue(string $className, object $metadata): void { diff --git a/test/Fixtures/NonStrictTypeConflictingDto.php b/test/Fixtures/NonStrictTypeConflictingDto.php new file mode 100644 index 0000000..4566d33 --- /dev/null +++ b/test/Fixtures/NonStrictTypeConflictingDto.php @@ -0,0 +1,39 @@ +assertValidationErrorResponse($response, ['arrayProperty', 'intProperty', 'floatProperty', 'boolProperty', 'stringProperty']); } + public function testValuesAreCoercedDuringAssignmentWhenDtoIsNotStrict(): void + { + self::bootKernel(); + $kernel = self::$kernel; + + // Empty request + $request = Request::create( + uri: '/_test_mixed', + method: 'POST', + server: [ + 'CONTENT_TYPE' => 'application/json', + ], + content: json_encode([ + 'intProperty' => '1', + 'floatProperty' => '1.2', + 'boolProperty' => '0', + 'stringProperty' => 1 + ], JSON_THROW_ON_ERROR), + ); + + $controller = new class { + public function __invoke(NonStrictTypeConflictingDto $dto): JsonResponse + { + return new JsonResponse(get_object_vars($dto)); + } + }; + + $request->attributes->set('_controller', $controller); + + $response = $kernel->handle($request); + + self::assertSame(200, $response->getStatusCode()); + $data = json_decode((string) $response->getContent(), true, 512, JSON_THROW_ON_ERROR); + self::assertArrayHasKey("intProperty", $data); + self::assertSame(1, $data["intProperty"]); + self::assertArrayHasKey("floatProperty", $data); + self::assertSame(1.2, $data["floatProperty"]); + self::assertArrayHasKey("boolProperty", $data); + self::assertSame(false, $data["boolProperty"]); + self::assertArrayHasKey("stringProperty", $data); + self::assertSame("1", $data["stringProperty"]); + } + /** * @param string[] $fields * diff --git a/test/Unit/Attribute/RequestDtoTest.php b/test/Unit/Attribute/RequestDtoTest.php new file mode 100644 index 0000000..6cc2f9b --- /dev/null +++ b/test/Unit/Attribute/RequestDtoTest.php @@ -0,0 +1,41 @@ +strict); + } + + public function testConstructorAcceptsStrictOption(): void + { + $instance = new RequestDto(strict: true); + self::assertTrue($instance->strict); + + $instance = new RequestDto(strict: false); + self::assertFalse($instance->strict); + } +} From 8cbb3ae755cbb6b490bb737469677221a631e5c3 Mon Sep 17 00:00:00 2001 From: Marvin Petker Date: Sun, 8 Feb 2026 20:52:45 +0100 Subject: [PATCH 6/6] refactor: rewrite integration tests with data providers and fix hydration edge cases - Add is_array guard in AbstractNestedParam, coerce non-array nested values in - RequestDtoFactory, and support QueryParam transformValue in fromArray path. - Replace individual test methods with parameterized data-provider tests covering - both fromRequest and fromArray, and add comprehensive fixture DTOs for strict, - mixed, non-strict, nested, and circular reference scenarios. --- phpstan.dist.neon | 4 - src/Attribute/AbstractNestedParam.php | 2 +- src/Factory/RequestDtoFactory.php | 13 +- .../Controller/DtoWithDefaultsController.php | 25 - .../MixedDtoWithDefaultsController.php | 2 +- .../MultipleFilesTestController.php | 2 +- .../Controller/StrictTypesDtoController.php | 2 +- test/Fixtures/Controller/TestController.php | 2 +- .../TypeConflictingDtoController.php | 2 +- test/Fixtures/{ => Dto}/AllParamTypesDTO.php | 2 +- .../{ => Dto}/AllTypesBodyParamsTrait.php | 13 +- .../{ => Dto}/CollectionPathTestDto.php | 2 +- .../DtoWithGroupSequenceProvider.php | 2 +- .../{ => Dto}/GroupSequenceProviderDTO.php | 2 +- test/Fixtures/{ => Dto}/Legacy/ExampleDto.php | 7 +- test/Fixtures/Dto/MixedAllParamsDto.php | 221 +++++++ .../{ => Dto}/MixedDtoWithDefaults.php | 2 +- test/Fixtures/{ => Dto}/MultipleFilesDto.php | 2 +- .../Nested}/CircularReferencingDto.php | 2 +- .../{ => Dto/Nested}/DeepNestedDto.php | 3 +- .../{ => Dto/Nested}/DtoWithChildDto.php | 7 +- .../Nested}/DtoWithNestedDtoArray.php | 4 +- .../Nested/NestedChildMixedDto.php} | 4 +- .../Nested/NestedChildStrictDto.php} | 4 +- .../Dto/New/CircularReferencingRequestDto.php | 30 + .../New/MixedType/MixedChildRequestDto.php} | 13 +- .../MixedType/MixedDeepNestedRequestDto.php | 36 ++ .../New/MixedType/MixedDtoWithChildDto.php | 42 ++ .../Dto/New/MixedType/MixedRequestDto.php | 218 +++++++ .../MixedRequestDtoWithDeepNesting.php | 36 ++ .../MixedRequestDtoWithShallowNesting.php | 37 ++ .../Dto/New/StrictTypes/ChildRequestDto.php | 26 + .../Dto/New/StrictTypes/DtoWithChildDto.php | 40 ++ .../Dto/New/StrictTypes/DtoWithDefaults.php | 47 ++ .../New/StrictTypes/NonStrictRequestDto.php | 112 ++++ .../StrictTypes/RequestDtoWithDeepNesting.php | 34 ++ .../RequestDtoWithShallowNesting.php | 33 + .../Dto/New/StrictTypes/StrictRequestDto.php | 112 ++++ .../Dto/New/TransformQueryParamRequestDto.php | 37 ++ .../{ => Dto}/NonStrictTypeConflictingDto.php | 2 +- test/Fixtures/Dto/StrictAllParamsDto.php | 160 +++++ test/Fixtures/{ => Dto}/StrictTypesDto.php | 2 +- .../Fixtures/{ => Dto}/TypeConflictingDto.php | 2 +- test/Fixtures/DtoWithDefaults.php | 107 ---- .../GroupProvider/TestGroupProvider.php | 2 +- .../RequestDtoFactoryIntegrationTest.php | 566 ++++++++++++++---- ...equestDtoResolverBundleIntegrationTest.php | 31 +- test/Unit/Utility/DtoReflectionHelperTest.php | 12 +- 48 files changed, 1758 insertions(+), 310 deletions(-) delete mode 100644 test/Fixtures/Controller/DtoWithDefaultsController.php rename test/Fixtures/{ => Dto}/AllParamTypesDTO.php (94%) rename test/Fixtures/{ => Dto}/AllTypesBodyParamsTrait.php (58%) rename test/Fixtures/{ => Dto}/CollectionPathTestDto.php (93%) rename test/Fixtures/{ => Dto}/DtoWithGroupSequenceProvider.php (92%) rename test/Fixtures/{ => Dto}/GroupSequenceProviderDTO.php (94%) rename test/Fixtures/{ => Dto}/Legacy/ExampleDto.php (86%) create mode 100644 test/Fixtures/Dto/MixedAllParamsDto.php rename test/Fixtures/{ => Dto}/MixedDtoWithDefaults.php (98%) rename test/Fixtures/{ => Dto}/MultipleFilesDto.php (93%) rename test/Fixtures/{ => Dto/Nested}/CircularReferencingDto.php (89%) rename test/Fixtures/{ => Dto/Nested}/DeepNestedDto.php (83%) rename test/Fixtures/{ => Dto/Nested}/DtoWithChildDto.php (75%) rename test/Fixtures/{ => Dto/Nested}/DtoWithNestedDtoArray.php (84%) rename test/Fixtures/{Legacy/NestedChildDTO.php => Dto/Nested/NestedChildMixedDto.php} (83%) rename test/Fixtures/{NestedChildDTO.php => Dto/Nested/NestedChildStrictDto.php} (83%) create mode 100644 test/Fixtures/Dto/New/CircularReferencingRequestDto.php rename test/Fixtures/{Legacy/UnionTypesDTO.php => Dto/New/MixedType/MixedChildRequestDto.php} (64%) create mode 100644 test/Fixtures/Dto/New/MixedType/MixedDeepNestedRequestDto.php create mode 100644 test/Fixtures/Dto/New/MixedType/MixedDtoWithChildDto.php create mode 100644 test/Fixtures/Dto/New/MixedType/MixedRequestDto.php create mode 100644 test/Fixtures/Dto/New/MixedType/MixedRequestDtoWithDeepNesting.php create mode 100644 test/Fixtures/Dto/New/MixedType/MixedRequestDtoWithShallowNesting.php create mode 100644 test/Fixtures/Dto/New/StrictTypes/ChildRequestDto.php create mode 100644 test/Fixtures/Dto/New/StrictTypes/DtoWithChildDto.php create mode 100644 test/Fixtures/Dto/New/StrictTypes/DtoWithDefaults.php create mode 100644 test/Fixtures/Dto/New/StrictTypes/NonStrictRequestDto.php create mode 100644 test/Fixtures/Dto/New/StrictTypes/RequestDtoWithDeepNesting.php create mode 100644 test/Fixtures/Dto/New/StrictTypes/RequestDtoWithShallowNesting.php create mode 100644 test/Fixtures/Dto/New/StrictTypes/StrictRequestDto.php create mode 100644 test/Fixtures/Dto/New/TransformQueryParamRequestDto.php rename test/Fixtures/{ => Dto}/NonStrictTypeConflictingDto.php (93%) create mode 100644 test/Fixtures/Dto/StrictAllParamsDto.php rename test/Fixtures/{ => Dto}/StrictTypesDto.php (94%) rename test/Fixtures/{ => Dto}/TypeConflictingDto.php (92%) delete mode 100644 test/Fixtures/DtoWithDefaults.php diff --git a/phpstan.dist.neon b/phpstan.dist.neon index a51a8d6..2bf8fb3 100644 --- a/phpstan.dist.neon +++ b/phpstan.dist.neon @@ -10,7 +10,3 @@ parameters: - src - test - ignoreErrors: - - - identifier: missingType.property - path: %currentWorkingDirectory%/test/Fixtures/Legacy diff --git a/src/Attribute/AbstractNestedParam.php b/src/Attribute/AbstractNestedParam.php index 4c712c5..ea1edae 100644 --- a/src/Attribute/AbstractNestedParam.php +++ b/src/Attribute/AbstractNestedParam.php @@ -54,7 +54,7 @@ public function hasValueInRequest(Request $request): bool : $this->getDataFromRequest($request) ; - return array_key_exists($this->getName(), $data); + return is_array($data) && array_key_exists($this->getName(), $data); } public function getValueFromRequest(Request $request): mixed diff --git a/src/Factory/RequestDtoFactory.php b/src/Factory/RequestDtoFactory.php index e875344..94f195b 100644 --- a/src/Factory/RequestDtoFactory.php +++ b/src/Factory/RequestDtoFactory.php @@ -216,6 +216,9 @@ private function createInstanceRecursive( $resultArray = []; foreach ($valueArray as $i => $nestedValue) { + if (!is_array($nestedValue)) { + continue; + } if ($isArray && $attr instanceof AbstractNestedParam) { $attr = clone $attr; $attr->setIndex($i); @@ -224,6 +227,7 @@ private function createInstanceRecursive( try { $resultArray[] = $this->createInstanceRecursive( $nestedClassName, + // @phpstan-ignore argument.type $context instanceof Request ? $context : $nestedValue, @@ -381,7 +385,14 @@ private function arrayValueProvider( RequestDtoParamMetadata $paramMetadata, array $data ): mixed { - return $data[$paramMetadata->getPropertyName()] ?? null; + $propertyName = $paramMetadata->getPropertyName(); + $value = $data[$propertyName] ?? null; + + if ($attr instanceof QueryParam) { + $value = $attr->transformValue($value); + } + + return $value; } /** diff --git a/test/Fixtures/Controller/DtoWithDefaultsController.php b/test/Fixtures/Controller/DtoWithDefaultsController.php deleted file mode 100644 index 11cdbcf..0000000 --- a/test/Fixtures/Controller/DtoWithDefaultsController.php +++ /dev/null @@ -1,25 +0,0 @@ - - */ - #[QueryParam] - #[Assert\NotBlank] - #[Assert\Type('array')] - #[Assert\All([ - new Assert\Type('string') - ])] - public array $queryParamArrayAssoc = ['key_1' => 'value_1', 'key_2' => 'value_2']; - - #[BodyParam] - #[Assert\NotBlank] - #[Assert\Type('string')] - public string $bodyParamString = 'body-param-string-default'; - #[BodyParam] - #[Assert\NotBlank] - #[Assert\Type('int')] - public int $bodyParamInt = 42; - #[BodyParam] - #[Assert\NotBlank] - #[Assert\Type('float')] - public float $bodyParamFloat = 3.14; - #[BodyParam] - #[Assert\NotBlank] - #[Assert\Type('bool')] - public bool $bodyParamBool = true; - - /** - * @var string[] - */ - #[BodyParam] - #[Assert\NotBlank] - #[Assert\Type('array')] - #[Assert\All([ - new Assert\Type('string') - ])] - public array $bodyParamArrayNueric = ['default', 'array']; - - /** - * @var array - */ - #[BodyParam] - #[Assert\NotBlank] - #[Assert\Type('array')] - #[Assert\All([ - new Assert\Type('string') - ])] - public array $bodyParamArrayAssoc = ['key_1' => 'value_1', 'key_2' => 'value_2']; - - #[HeaderParam('X-Custom-Header')] - #[Assert\NotBlank] - #[Assert\Type('string')] - public string $headerParamString = 'header-param-string-default'; -} diff --git a/test/Fixtures/GroupProvider/TestGroupProvider.php b/test/Fixtures/GroupProvider/TestGroupProvider.php index 1d52bb5..8d75bc8 100644 --- a/test/Fixtures/GroupProvider/TestGroupProvider.php +++ b/test/Fixtures/GroupProvider/TestGroupProvider.php @@ -13,7 +13,7 @@ namespace Crtl\RequestDtoResolverBundle\Test\Fixtures\GroupProvider; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\DtoWithGroupSequenceProvider; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\DtoWithGroupSequenceProvider; use Symfony\Component\Validator\Constraints\GroupSequence; use Symfony\Component\Validator\GroupProviderInterface; diff --git a/test/Integration/RequestDtoFactoryIntegrationTest.php b/test/Integration/RequestDtoFactoryIntegrationTest.php index 22e87c8..0ea5b97 100644 --- a/test/Integration/RequestDtoFactoryIntegrationTest.php +++ b/test/Integration/RequestDtoFactoryIntegrationTest.php @@ -16,13 +16,33 @@ use Crtl\RequestDtoResolverBundle\Factory\Exception\CircularReferenceException; use Crtl\RequestDtoResolverBundle\Factory\Exception\RequestDtoHydrationException; use Crtl\RequestDtoResolverBundle\Factory\RequestDtoFactory; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\AllParamTypesDTO; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\CircularReferencingDto; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\DtoWithNestedDtoArray; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\NestedChildDTO; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\New\CircularReferencingRequestDto; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\New\MixedType\MixedRequestDto; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\New\StrictTypes\NonStrictRequestDto; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\New\StrictTypes\RequestDtoWithDeepNesting; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\New\StrictTypes\RequestDtoWithShallowNesting; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\New\StrictTypes\StrictRequestDto; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\New\TransformQueryParamRequestDto; +use PHPUnit\Framework\Attributes\DataProvider; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Validator\ConstraintViolationInterface; +use Symfony\Component\Validator\ConstraintViolationListInterface; +/** + * @phpstan-type HydrationTestCaseArgs array{ + * method: string, + * context: array|Request, + * expected: array, + * className: class-string, + * } + * @phpstan-type ViolationTestCaseArgs array{ + * method: string, + * context: array|Request, + * expected: string[], + * className: class-string, + * } + */ final class RequestDtoFactoryIntegrationTest extends KernelTestCase { private RequestDtoFactory $factory; @@ -36,146 +56,488 @@ protected function setUp(): void $this->factory = $factory; } - public function testFromRequestThrowsReflectionExceptionWhenClassDoesNotExist(): void + /** + * @return iterable|Request}> + */ + public static function throwsReflectionExceptionWhenClassDoesNotExist(): iterable { - $this->expectException(\ReflectionException::class); - // @phpstan-ignore argument.type - $this->factory->fromRequest('NonExistingClass', new Request()); + yield 'fromRequest' => ['fromRequest', new Request()]; + yield 'fromArray' => ['fromArray', []]; } - public function testFromRequestHydratesAllParamTypes(): void + /** + * @param array|Request $context + */ + #[DataProvider('throwsReflectionExceptionWhenClassDoesNotExist')] + public function testThrowsReflectionExceptionWhenClassDoesNotExist(string $method, array|Request $context): void { - $request = new Request( - query: ['query' => 'query_val'], - request: ['body' => 'body_val'], - attributes: ['_route_params' => ['_route' => 'test_route']], - server: ['HTTP_USER_AGENT' => 'test_ua'], - ); + $this->expectException(\ReflectionException::class); + $this->factory->$method('NonExistingClass', $context); + } - /** @var AllParamTypesDTO $dto */ - $dto = $this->factory->fromRequest(AllParamTypesDTO::class, $request); + /** + * @return iterable|Request}> + */ + public static function throwsCircularReferenceExceptionWhenNestedCircularReferenceIsDetectedProvider(): iterable + { + yield 'fromArray with single child' => [ + 'method' => 'fromArray', + 'context' => ['prop' => []], + ]; + yield 'fromArray with array child' => [ + 'method' => 'fromArray', + 'context' => ['array' => [[]]], + ]; + yield 'fromRequest with single child' => [ + 'method' => 'fromRequest', + 'context' => new Request(request: ['prop' => []]), + ]; + yield 'fromRequest with array child' => [ + 'method' => 'fromRequest', + 'context' => new Request(request: ['array' => [[]]]), + ]; + } - // @phpstan-ignore method.alreadyNarrowedType - $this->assertInstanceOf(AllParamTypesDTO::class, $dto); - $this->assertEquals('query_val', $dto->query); - $this->assertEquals('body_val', $dto->body); - $this->assertEquals('test_route', $dto->routeName); - $this->assertEquals('test_ua', $dto->userAgent); + /** + * @param array|Request $context + */ + #[DataProvider('throwsCircularReferenceExceptionWhenNestedCircularReferenceIsDetectedProvider')] + public function testThrowsCircularReferenceExceptionWhenNestedCircularReferenceIsDetected(string $method, array|Request $context): void + { + self::expectException(CircularReferenceException::class); + $this->factory->$method(CircularReferencingRequestDto::class, $context); } - public function testFromRequestHydratesNestedDtos(): void + /** + * @return iterable + */ + public static function correctlyHydratesRequestDtosProvider(): iterable { - $request = new Request( - request: [ + // TODO: implement data providers, optionally add more entries. + yield 'mixed fromRequest' => [ + 'method' => 'fromRequest', + 'context' => new Request(request: ['prop' => []]), + 'expected' => [], + 'className' => MixedRequestDto::class, + ]; + yield 'mixed fromArray' => [ + 'method' => 'fromArray', + 'context' => [], + 'expected' => [], + 'className' => MixedRequestDto::class, + ]; + + // TODO: implement data providers, optionally add more entries. + yield 'strict fromRequest' => [ + 'method' => 'fromRequest', + 'context' => new Request(request: ['prop' => []]), + 'expected' => [], + 'className' => StrictRequestDto::class, + ]; + yield 'strict fromArray' => [ + 'method' => 'fromArray', + 'context' => [], + 'expected' => [], + 'className' => StrictRequestDto::class, + ]; + + // TODO: implement data provider + yield 'non-strict fromRequest' => [ + 'method' => 'fromRequest', + 'context' => new Request(request: ['prop' => []]), + 'expected' => [], + 'className' => NonStrictRequestDto::class, + ]; + yield 'non-strict fromArray' => [ + 'method' => 'fromArray', + 'context' => [], + 'expected' => [], + 'className' => NonStrictRequestDto::class, + ]; + + yield 'deep nested fromRequest with single child' => [ + 'method' => 'fromRequest', + 'context' => new Request(request: [ + 'child' => [ + 'child' => [ + 'childName' => 'name', + ] + ] + ]), + 'expected' => [ + 'child' => [ + 'child' => [ + 'childName' => 'name', + ] + ] + ], + 'className' => RequestDtoWithDeepNesting::class, + ]; + yield 'deep nested fromRequest with multiple children' => [ + 'method' => 'fromRequest', + 'context' => new Request(request: [ + 'children' => [ + [ + 'children' => [[ + 'childName' => 'name', + ]] + ] + ] + ]), + 'expected' => [ 'children' => [ - ['childName' => 'child1'], - ['childName' => 'child2'], + [ + 'children' => [[ + 'childName' => 'name', + ]] + ] ] ], - ); - - /** @var DtoWithNestedDtoArray $dto */ - $dto = $this->factory->fromRequest(DtoWithNestedDtoArray::class, $request); - - // @phpstan-ignore method.alreadyNarrowedType - $this->assertInstanceOf(DtoWithNestedDtoArray::class, $dto); - $this->assertCount(2, $dto->children); + 'className' => RequestDtoWithDeepNesting::class, + ]; + yield 'deep nested fromArray with single child' => [ + 'method' => 'fromArray', + 'context' => [ + 'child' => [ + 'child' => [ + 'childName' => 'name', + ] + ] + ], + 'expected' => [ + 'child' => [ + 'child' => [ + 'childName' => 'name', + ] + ] + ], + 'className' => RequestDtoWithDeepNesting::class, + ]; + yield 'deep nested fromArray with multiple children' => [ + 'method' => 'fromArray', + 'context' => [ + 'children' => [ + [ + 'children' => [[ + 'childName' => 'name', + ]] + ] + ] + ], + 'expected' => [ + 'children' => [ + [ + 'children' => [[ + 'childName' => 'name', + ]] + ] + ] + ], + 'className' => RequestDtoWithDeepNesting::class, + ]; - // @phpstan-ignore method.alreadyNarrowedType - $this->assertInstanceOf(NestedChildDTO::class, $dto->children[0]); - $this->assertEquals('child1', $dto->children[0]->childName); + yield 'shallow nested fromRequest with single child' => [ + 'method' => 'fromRequest', + 'context' => new Request(request: [ + 'child' => [ + 'childName' => 'name', + ] + ]), + 'expected' => [ + 'child' => [ + 'childName' => 'name', + ] + ], + 'className' => RequestDtoWithShallowNesting::class, + ]; + yield 'shallow nested fromRequest with multiple children' => [ + 'method' => 'fromRequest', + 'context' => new Request(request: [ + 'children' => [[ + 'childName' => 'name', + ]] + ]), + 'expected' => [ + 'children' => [[ + 'childName' => 'name', + ]] + ], + 'className' => RequestDtoWithShallowNesting::class, + ]; + yield 'shallow nested fromArray with single child' => [ + 'method' => 'fromArray', + 'context' => [ + 'child' => [ + 'childName' => 'name', + ] + ], + 'expected' => [ + 'child' => [ + 'childName' => 'name', + ] + ], + 'className' => RequestDtoWithShallowNesting::class, + ]; + yield 'shallow nested fromArray with multiple children' => [ + 'method' => 'fromArray', + 'context' => [ + 'children' => [[ + 'childName' => 'name', + ]] + ], + 'expected' => [ + 'children' => [[ + 'childName' => 'name', + ]] + ], + 'className' => RequestDtoWithShallowNesting::class, + ]; - // @phpstan-ignore method.alreadyNarrowedType - $this->assertInstanceOf(NestedChildDTO::class, $dto->children[1]); - $this->assertEquals('child2', $dto->children[1]->childName); + yield 'transform query param fromRequest' => [ + 'method' => 'fromRequest', + 'context' => new Request([ + 'queryInt' => '123', + 'queryNullableInt' => '321', + 'queryFloat' => '1.2', + 'queryNullableFloat' => '3.14', + 'queryBool' => 'yes', + 'queryNullableBool' => 'false', + ]), + 'expected' => [ + 'queryInt' => 123, + 'queryNullableInt' => 321, + 'queryFloat' => 1.2, + 'queryNullableFloat' => 3.14, + 'queryBool' => true, + 'queryNullableBool' => false, + ], + 'className' => TransformQueryParamRequestDto::class, + ]; + yield 'transform query param fromArray' => [ + 'method' => 'fromArray', + 'context' => [ + 'queryInt' => '123', + 'queryNullableInt' => '321', + 'queryFloat' => '1.2', + 'queryNullableFloat' => '3.14', + 'queryBool' => 'yes', + 'queryNullableBool' => 'false', + ], + 'expected' => [ + 'queryInt' => 123, + 'queryNullableInt' => 321, + 'queryFloat' => 1.2, + 'queryNullableFloat' => 3.14, + 'queryBool' => true, + 'queryNullableBool' => false, + ], + 'className' => TransformQueryParamRequestDto::class, + ]; } - public function testFromArrayHydratesDto(): void + /** + * @param array|Request $context + * @param array $expected + * @param class-string $className + */ + #[DataProvider('correctlyHydratesRequestDtosProvider')] + public function testCorrectlyHydratesRequestDtos( + string $method, + array|Request $context, + array $expected, + string $className, + ): void { - $data = [ - 'query' => 'q', - 'body' => 'b', - 'routeName' => 'r', - 'userAgent' => 'ua' - ]; - - /** @var AllParamTypesDTO $dto */ - $dto = $this->factory->fromArray(AllParamTypesDTO::class, $data); - - // @phpstan-ignore method.alreadyNarrowedType - $this->assertInstanceOf(AllParamTypesDTO::class, $dto); - $this->assertEquals('q', $dto->query); - $this->assertEquals('b', $dto->body); - $this->assertEquals('r', $dto->routeName); - $this->assertEquals('ua', $dto->userAgent); + $dto = $this->factory->$method($className, $context); + self::assertInstanceOf($className, $dto); + self::objectMatchesExpectedStructure($dto, $expected); } - public function testFromArrayThrowsExceptionOnTypeError(): void + /** + * @return iterable + */ + public static function throwsRequestDtoHydrationExceptionWithPropertyTypeViolationsWhenHydratingTypedDtoWithInvalidTypesProvider(): iterable { - $data = [ - 'childName' => ['invalid'], // Expected string, got array + yield 'fromArray with invalid body scalar types' => [ + 'method' => 'fromArray', + 'context' => [ + 'bodyString' => ['not', 'a', 'string'], + 'bodyInt' => 'not-a-number', + 'bodyFloat' => 'not-a-number', + 'bodyBool' => ['not-a-bool'], + 'bodyArray' => 'not-an-array', + ], + 'expected' => ['bodyString', 'bodyInt', 'bodyFloat', 'bodyBool', 'bodyArray'], + 'className' => StrictRequestDto::class, ]; - $this->expectException(RequestDtoHydrationException::class); + yield 'fromRequest with invalid body scalar types' => [ + 'method' => 'fromRequest', + 'context' => new Request(request: [ + 'bodyString' => ['not', 'a', 'string'], + 'bodyInt' => 'not-a-number', + 'bodyFloat' => 'not-a-number', + 'bodyBool' => ['not-a-bool'], + 'bodyArray' => 'not-an-array', + ]), + 'expected' => ['bodyString', 'bodyInt', 'bodyFloat', 'bodyBool', 'bodyArray'], + 'className' => StrictRequestDto::class, + ]; - try { - $this->factory->fromArray(NestedChildDTO::class, $data); - } catch (RequestDtoHydrationException $e) { - $this->assertCount(1, $e->violations); - // The violation path is prefixed with the property name in fromArrayRecursive - $this->assertEquals('childName', $e->violations[0]->getPropertyPath()); - throw $e; - } + yield 'fromArray with single invalid property among valid ones' => [ + 'method' => 'fromArray', + 'context' => [ + 'bodyString' => 'valid-string', + 'bodyInt' => ['not-an-int'], + ], + 'expected' => ['bodyInt'], + 'className' => StrictRequestDto::class, + ]; + + yield 'fromArray with invalid types across param sources' => [ + 'method' => 'fromArray', + 'context' => [ + 'bodyInt' => ['not-an-int'], + ], + 'expected' => ['bodyInt'], + 'className' => StrictRequestDto::class, + ]; + + yield 'fromRequest with invalid types across param sources' => [ + 'method' => 'fromRequest', + 'context' => new Request( + query: ['queryString' => ['not-a-string']], + request: ['bodyInt' => ['not-an-int']], + ), + 'expected' => ['bodyInt', 'queryString'], + 'className' => StrictRequestDto::class, + ]; + + yield 'fromArray with null for non-nullable properties' => [ + 'method' => 'fromArray', + 'context' => [ + 'bodyString' => null, + 'bodyInt' => null, + ], + 'expected' => ['bodyString', 'bodyInt'], + 'className' => StrictRequestDto::class, + ]; + + yield 'fromArray with deep nested dto' => [ + 'method' => 'fromArray', + 'context' => [ + 'child' => [ + 'child' => [ + 'childName' => 1, + ], + 'children' => [ + ['childName' => 1], + ] + ], + 'children' => [ + [ + 'child' => [ + 'childName' => 1, + ], + 'children' => [ + ['childName' => 1], + null, + ] + ], + ], + ], + 'expected' => [ + 'child.child.childName', + 'child.children[0].childName', + 'children[0].child.childName', + 'children[0].children[0].childName', + ], + 'className' => RequestDtoWithDeepNesting::class, + ]; } - public function testFromArrayThrowsExceptionWithNestedArrayRequestDtoTypeMismatch(): void + /** + * @param array|Request $context + * @param string[] $expectedViolations + * @param class-string $className + */ + #[DataProvider('throwsRequestDtoHydrationExceptionWithPropertyTypeViolationsWhenHydratingTypedDtoWithInvalidTypesProvider')] + public function testThrowsRequestDtoHydrationExceptionWithPropertyTypeViolationsWhenHydratingTypedDtoWithInvalidTypes( + string $method, + array|Request $context, + array $expectedViolations, + string $className, + ): void { self::expectException(RequestDtoHydrationException::class); try { - $this->factory->fromArray( - DtoWithNestedDtoArray::class, - ['children' => [ - [], // No name at all an property is not nullable - ['childName' => 1], // Int despite string is expected - ['childName' => 1.0], // float despite string is expected - ['childName' => false], // bool despite string is expected - ['childName' => new \stdClass()], // object despite string is expected - ]], - ); + $this->factory->$method($className, $context); } catch (RequestDtoHydrationException $e) { - $this->assertCount(4, $e->violations); - self::assertSame('children[1].childName', $e->violations[0]->getPropertyPath()); - self::assertSame('children[2].childName', $e->violations[1]->getPropertyPath()); - self::assertSame('children[3].childName', $e->violations[2]->getPropertyPath()); - self::assertSame('children[4].childName', $e->violations[3]->getPropertyPath()); - + self::assertRequestDtoHydrationException($e, $expectedViolations); throw $e; } } - public function testFromArrayThrowsCircularReferenceExceptionWhenCircularReferenceIsDetected(): void + /** + * @return array + */ + private static function mapViolationsToPath(ConstraintViolationListInterface $violations): array { - self::expectException(CircularReferenceException::class); - $this->factory->fromArray(CircularReferencingDto::class, ['prop' => []]); + $result = []; + foreach ($violations as $violation) { + $path = $violation->getPropertyPath(); + $result[$path] ??= []; + $result[$path][] = $violation; + } + + return $result; } - public function testFromArrayThrowsCircularReferenceExceptionWhenNestedCircularReferenceIsDetected(): void + /** + * @template TKey of array-key + * @template TVal + * + * @param array $keys + * @param array $array + */ + private static function assertArrayHasKeys(array $keys, array $array): void { - self::expectException(CircularReferenceException::class); - $this->factory->fromArray(CircularReferencingDto::class, ['array' => [[]]]); + foreach ($keys as $key) { + self::assertArrayHasKey($key, $array); + } } - public function testFromRequestThrowsCircularReferenceExceptionWhenCircularReferenceIsDetected(): void + /** + * Helper to assert that error contains violations for given fields. + * + * @param string[]|array $expected property paths of expected violations or a map of field names to expected violation count + */ + private static function assertRequestDtoHydrationException(RequestDtoHydrationException $e, array $expected): void { - self::expectException(CircularReferenceException::class); - $request = new Request(request: ['prop' => []]); - $this->factory->fromRequest(CircularReferencingDto::class, $request); + $violations = self::mapViolationsToPath($e->violations); + self::assertCount(count($expected), $violations); + foreach ($expected as $key => $value) { + if (is_int($key)) { + $key = $value; + } + + self::assertArrayHasKey($key, $violations); + + // Only assert count when array is assoc + if (is_int($value)) { // @phpstan-ignore-line + self::assertCount($value, $violations[$key]); // @phpstan-ignore-line + } + } } - public function testFromRequestThrowsCircularReferenceExceptionWhenNestedCircularReferenceIsDetected(): void + /** + * @param array $expected + */ + private static function objectMatchesExpectedStructure(object $object, array $expected): void { - self::expectException(CircularReferenceException::class); - $request = new Request(request: ['array' => [[]]]); - $this->factory->fromRequest(CircularReferencingDto::class, $request); + self::assertSame( + $expected, + json_decode(json_encode($object) ?: '', true), + ); } } diff --git a/test/Integration/RequestDtoResolverBundleIntegrationTest.php b/test/Integration/RequestDtoResolverBundleIntegrationTest.php index 1c42601..3d21d54 100644 --- a/test/Integration/RequestDtoResolverBundleIntegrationTest.php +++ b/test/Integration/RequestDtoResolverBundleIntegrationTest.php @@ -13,16 +13,16 @@ namespace Crtl\RequestDtoResolverBundle\Test\Integration; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\CollectionPathTestDto; use Crtl\RequestDtoResolverBundle\Test\Fixtures\Controller\MixedDtoWithDefaultsController; use Crtl\RequestDtoResolverBundle\Test\Fixtures\Controller\MultipleFilesTestController; use Crtl\RequestDtoResolverBundle\Test\Fixtures\Controller\StrictTypesDtoController; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\DtoWithGroupSequenceProvider; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\DtoWithNestedDtoArray; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\GroupSequenceProviderDTO; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\Legacy\ExampleDto; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\NonStrictTypeConflictingDto; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\TypeConflictingDto; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\CollectionPathTestDto; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\DtoWithGroupSequenceProvider; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\GroupSequenceProviderDTO; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\Legacy\ExampleDto; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\Nested\DtoWithNestedDtoArray; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\NonStrictTypeConflictingDto; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\TypeConflictingDto; use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\HttpFoundation\JsonResponse; @@ -531,7 +531,6 @@ public function testMixedDtoWithDefaultsIsHydratedCorrectlyAndRetainsDefaults(): $response = $kernel->handle($request); $data = json_decode((string) $response->getContent(), true, 512, JSON_THROW_ON_ERROR); - print_r($data); self::assertSame(200, $response->getStatusCode()); // Provided values @@ -662,14 +661,14 @@ public function __invoke(NonStrictTypeConflictingDto $dto): JsonResponse self::assertSame(200, $response->getStatusCode()); $data = json_decode((string) $response->getContent(), true, 512, JSON_THROW_ON_ERROR); - self::assertArrayHasKey("intProperty", $data); - self::assertSame(1, $data["intProperty"]); - self::assertArrayHasKey("floatProperty", $data); - self::assertSame(1.2, $data["floatProperty"]); - self::assertArrayHasKey("boolProperty", $data); - self::assertSame(false, $data["boolProperty"]); - self::assertArrayHasKey("stringProperty", $data); - self::assertSame("1", $data["stringProperty"]); + self::assertArrayHasKey('intProperty', $data); + self::assertSame(1, $data['intProperty']); + self::assertArrayHasKey('floatProperty', $data); + self::assertSame(1.2, $data['floatProperty']); + self::assertArrayHasKey('boolProperty', $data); + self::assertFalse($data['boolProperty']); + self::assertArrayHasKey('stringProperty', $data); + self::assertSame('1', $data['stringProperty']); } /** diff --git a/test/Unit/Utility/DtoReflectionHelperTest.php b/test/Unit/Utility/DtoReflectionHelperTest.php index f876b00..72718c7 100644 --- a/test/Unit/Utility/DtoReflectionHelperTest.php +++ b/test/Unit/Utility/DtoReflectionHelperTest.php @@ -19,8 +19,8 @@ use Crtl\RequestDtoResolverBundle\Attribute\QueryParam; use Crtl\RequestDtoResolverBundle\Attribute\RequestDto; use Crtl\RequestDtoResolverBundle\Attribute\RouteParam; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\AllParamTypesDTO; -use Crtl\RequestDtoResolverBundle\Test\Fixtures\NestedChildDTO; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\AllParamTypesDTO; +use Crtl\RequestDtoResolverBundle\Test\Fixtures\Dto\Nested\NestedChildStrictDto; use Crtl\RequestDtoResolverBundle\Utility\DtoReflectionHelper; use PHPUnit\Framework\TestCase; @@ -37,7 +37,7 @@ public function testGetDtoClassNameFromReflectionProperty(): void { $class = new #[RequestDto] class { #[BodyParam] - public ?NestedChildDTO $nested; + public ?NestedChildStrictDto $nested; #[BodyParam] public ?string $notDto; @@ -51,7 +51,7 @@ public function testGetDtoClassNameFromReflectionProperty(): void $property = $reflectionClass->getProperty('nested'); $type = $this->helper->getDtoClassNameFromReflectionProperty($property); $this->assertNotNull($type); - $this->assertEquals(NestedChildDTO::class, $type->getName()); + $this->assertEquals(NestedChildStrictDto::class, $type->getName()); $property = $reflectionClass->getProperty('notDto'); $type = $this->helper->getDtoClassNameFromReflectionProperty($property); @@ -67,14 +67,14 @@ public function testGetDtoClassNameFromReflectionPropertyThrowsRuntimeExceptionF $unionClass = new #[RequestDto] class { #[BodyParam] - public NestedChildDTO|string $unionProperty; + public NestedChildStrictDto|string $unionProperty; }; $intersectionClass = new #[RequestDto] class { #[BodyParam] // @phpstan-ignore property.unresolvableNativeType - public NestedChildDTO&\Stringable $intersectionProperty; + public NestedChildStrictDto&\Stringable $intersectionProperty; }; $unionReflectionClass = new \ReflectionClass($unionClass);