Fix generated fast path bypassing global TransformerRegistry casters - #114
Conversation
`AbstractDto::fromArray()` only checked `HAS_FAST_PATH`, unlike the constructor and `_toArrayInternal()`, so globally registered casters were silently skipped when hydrating a generated DTO via `fromArray()` while `new Dto($data)` honored them. Scope the guards to the relevant direction as well: input paths bypass the fast path only when a caster is registered, output only when a serializer is registered. Registering a serializer no longer disables fast-path hydration (and vice versa), which matters for DTOs with lazy fields. Adds `TransformerRegistry::hasAnyCaster()` and `hasAnySerializer()`; `hasAny()` is kept.
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness issue where generated DTOs using the hydration fast path (HAS_FAST_PATH) could bypass globally registered casters when calling AbstractDto::fromArray(). It also refines the fast-path guard conditions so that fast paths are bypassed only in the relevant direction: casters disable fast-path hydration, while serializers disable fast-path serialization.
Changes:
- Add
TransformerRegistry::hasAnyCaster()andTransformerRegistry::hasAnySerializer()to distinguish input vs output transformer registrations. - Update
Dto::__construct(),AbstractDto::fromArray(), andDto::_toArrayInternal()to use the new direction-specific guards. - Add a fast-path-like test DTO plus new tests to cover caster/serializer interactions with generated fast paths.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/TestDto/FastPathCasterDto.php | Adds a test DTO that mimics generated fast-path behavior for a class-typed field. |
| tests/Dto/DtoTest.php | Adds regression tests ensuring global casters/serializers interact correctly with fast-path DTO hydration/serialization. |
| src/Transformer/TransformerRegistry.php | Adds hasAnyCaster() / hasAnySerializer() helpers while keeping hasAny() intact. |
| src/Dto/Dto.php | Switches fast-path guards: constructor checks casters; _toArrayInternal() checks serializers. |
| src/Dto/AbstractDto.php | Ensures fromArray() fast path is only taken when no global casters are registered. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #114 +/- ##
============================================
+ Coverage 83.07% 83.27% +0.19%
- Complexity 1555 1558 +3
============================================
Files 45 45
Lines 3835 3839 +4
============================================
+ Hits 3186 3197 +11
+ Misses 649 642 -7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AbstractDto::fromArray()guarded the generated fast path withHAS_FAST_PATHonly, while__construct()and_toArrayInternal()additionally checked theTransformerRegistry. Casters are consumed in the genericsetFromArray()path, so a globally registered caster was silently skipped when hydrating a generated DTO throughfromArray(), even thoughnew Dto($data)applied it.While fixing this, the guards are also scoped to the direction they actually apply to:
fromArray()) bypasses the fast path only when a caster is registeredtoArray()) bypasses it only when a serializer is registeredPreviously any registration disabled both, so registering an output-only serializer forced generic hydration - an unnecessary slowdown, and a behavior change for DTOs with lazy fields, which would then hydrate eagerly.
TransformerRegistry::hasAnyCaster()andhasAnySerializer()are added for this;hasAny()stays as is.The regression was untestable with the existing fixtures because they are all non-fast-path, so
tests/TestDto/FastPathCasterDto.phpmimics generated output for a class-typed field.