diff --git a/AGENTS.md b/AGENTS.md index 8221967..3c99bca 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -206,7 +206,7 @@ List changed API surfaces and relevant attributes/converters in the “Reference ## 7) Domain cheat sheet * **Property Info:** Use Symfony’s extractor stack (reflection, PHPDoc, type info). Respect the ordering and fallbacks. -* **Type handlers:** Implement `MagicSunday\JsonMapper\Value\TypeHandlerInterface`; handlers are stateless and must honour the `supports()`/`convert()` contract. +* **Type handlers:** Implement `MagicSunday\JsonMapper\Value\TypeHandlerInterface`; handlers are stateless and must honour the `supports()`/`convert()` contract. This is the ONE public extension point for value conversion. The `Value\Strategy\*` classes and `ValueConversionStrategyInterface` are `@internal` - the conversion chain's order is an implementation detail, not a contract, and there is no public hook to register a strategy. A handler runs ahead of the built-in deciding strategies but AFTER null handling, so it never sees a null. Their `supports()` carries a `MappingContext` the handler contract does not; the two are not meant to align, since one is internal and one is public. * **Attributes:** * `ReplaceNullWithDefaultValue` — only applies when a default exists. * `ReplaceProperty` — supports multiple alias names. They are collected into a map keyed by diff --git a/docs/API.md b/docs/API.md index 3dd944d..7a91e46 100644 --- a/docs/API.md +++ b/docs/API.md @@ -50,6 +50,12 @@ var_dump($mapper::class); ### Methods +> The built-in `Value\Strategy\*` classes are `@internal`: they are the conversion chain's +> implementation, not an extension point, and there is no public way to register one. A +> `TypeHandlerInterface` added through `addTypeHandler()` is consulted ahead of the built-in +> *deciding* strategies (after null handling), which is the supported way to change how a type +> converts. + | Method | Description | | --- | --- | | `addTypeHandler(TypeHandlerInterface $handler): self` | Registers a reusable conversion strategy for a specific type. | diff --git a/src/JsonMapper.php b/src/JsonMapper.php index 5bb33e8..efa91f0 100644 --- a/src/JsonMapper.php +++ b/src/JsonMapper.php @@ -277,12 +277,12 @@ public function addType(string $type, Closure $closure): JsonMapper * $allowedTargets drops the list the earlier registration carried, since a list written for one * closure must not outlive it. Registration order therefore decides what is enforced. * - * @param class-string $className Fully qualified class name that should be resolved dynamically. - * @param Closure(mixed):class-string|Closure(mixed, MappingContext):class-string $closure Closure that returns the concrete class to instantiate for the provided value. - * @param list|null $allowedTargets Classes the closure may return; null leaves it unrestricted. + * @param class-string $className Fully qualified class name that should be resolved dynamically. + * @param Closure(mixed):class-string|Closure(mixed, MappingContext):class-string|class-string $closure Closure returning the concrete class for the value, or a plain class-string to map to unconditionally. + * @param list|null $allowedTargets Classes the closure may return; null leaves it unrestricted. Only meaningful for a closure - a static class-string target has no payload-derived choice to constrain. * * @phpstan-param class-string $className - * @phpstan-param Closure(mixed):class-string|Closure(mixed, MappingContext):class-string $closure + * @phpstan-param Closure(mixed):class-string|Closure(mixed, MappingContext):class-string|class-string $closure * * @return JsonMapper Returns the mapper instance for fluent configuration. * @@ -290,7 +290,7 @@ public function addType(string $type, Closure $closure): JsonMapper */ public function addCustomClassMapEntry( string $className, - Closure $closure, + Closure|string $closure, ?array $allowedTargets = null, ): JsonMapper { $this->classResolver->add($className, $closure, $allowedTargets); diff --git a/src/JsonMapper/Resolver/ClassResolver.php b/src/JsonMapper/Resolver/ClassResolver.php index be14103..4d2b6a9 100644 --- a/src/JsonMapper/Resolver/ClassResolver.php +++ b/src/JsonMapper/Resolver/ClassResolver.php @@ -65,21 +65,35 @@ public function __construct(array $classMap = []) /** * Adds a custom resolution rule. * - * @param class-string $className Base class or interface the resolver handles. - * @param Closure(mixed):class-string|Closure(mixed, MappingContext):class-string $resolver Callback returning a concrete class based on the JSON payload and optional mapping context. - * @param list|null $allowedTargets Classes the resolver may return. Null leaves it - * unrestricted, which is the default for backwards - * compatibility - see the note below. + * The target may be a resolver closure or a plain class-string, matching what the constructor + * $classMap already accepts - a static mapping (SdkFoo::class => Foo::class) could be given at + * construction but not at runtime without wrapping it in a trivial closure. resolve() already + * handles both. + * + * @param class-string $className Base class or interface the resolver handles. + * @param Closure(mixed):class-string|Closure(mixed, MappingContext):class-string|class-string $resolver Resolver closure, or a concrete class-string to map to unconditionally. + * @param list|null $allowedTargets Classes the resolver may return. Null leaves it + * unrestricted, which is the default for backwards + * compatibility - see the note below. Only + * meaningful for a closure: a static string target + * is returned directly, with no payload choice to + * constrain, so a list beside one is inert. * * @phpstan-param class-string $className - * @phpstan-param Closure(mixed):class-string|Closure(mixed, MappingContext):class-string $resolver + * @phpstan-param Closure(mixed):class-string|Closure(mixed, MappingContext):class-string|class-string $resolver * - * @throws DomainException When the base class or a listed target does not exist. + * @throws DomainException When the base class, the target, or a listed target does not exist. */ - public function add(string $className, Closure $resolver, ?array $allowedTargets = null): void + public function add(string $className, Closure|string $resolver, ?array $allowedTargets = null): void { $this->assertClassString($className); + // A plain class-string target is validated on the way in, like the constructor does, so a + // typo fails at registration rather than at the first payload. + if (is_string($resolver)) { + $this->assertClassString($resolver); + } + // Everything is validated BEFORE anything is stored, so a rejected list cannot leave the // resolver registered without it. Storing first made this fail OPEN: a typo in the list // threw, and the entry it was meant to restrict stayed live and unrestricted - the exact diff --git a/src/JsonMapper/Value/Strategy/BuiltinValueConversionStrategy.php b/src/JsonMapper/Value/Strategy/BuiltinValueConversionStrategy.php index 2d7e192..5a876cc 100644 --- a/src/JsonMapper/Value/Strategy/BuiltinValueConversionStrategy.php +++ b/src/JsonMapper/Value/Strategy/BuiltinValueConversionStrategy.php @@ -41,6 +41,9 @@ /** * Converts scalar values to the requested builtin type. + * + * @internal This is not a public extension point. Register conversions through + * {@see \MagicSunday\JsonMapper\Value\TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ final class BuiltinValueConversionStrategy implements ValueConversionStrategyInterface { diff --git a/src/JsonMapper/Value/Strategy/CollectionValueConversionStrategy.php b/src/JsonMapper/Value/Strategy/CollectionValueConversionStrategy.php index 83fcdb1..c3b779f 100644 --- a/src/JsonMapper/Value/Strategy/CollectionValueConversionStrategy.php +++ b/src/JsonMapper/Value/Strategy/CollectionValueConversionStrategy.php @@ -30,6 +30,9 @@ /** * Converts collection values using the configured factory. + * + * @internal This is not a public extension point. Register conversions through + * {@see \MagicSunday\JsonMapper\Value\TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ final readonly class CollectionValueConversionStrategy implements ValueConversionStrategyInterface { diff --git a/src/JsonMapper/Value/Strategy/CustomTypeValueConversionStrategy.php b/src/JsonMapper/Value/Strategy/CustomTypeValueConversionStrategy.php index 0e1a835..4a67aa1 100644 --- a/src/JsonMapper/Value/Strategy/CustomTypeValueConversionStrategy.php +++ b/src/JsonMapper/Value/Strategy/CustomTypeValueConversionStrategy.php @@ -17,6 +17,9 @@ /** * Handles conversion of registered custom types. + * + * @internal This is not a public extension point. Register conversions through + * {@see \MagicSunday\JsonMapper\Value\TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ final readonly class CustomTypeValueConversionStrategy implements ValueConversionStrategyInterface { diff --git a/src/JsonMapper/Value/Strategy/DateTimeValueConversionStrategy.php b/src/JsonMapper/Value/Strategy/DateTimeValueConversionStrategy.php index 7365e8d..7e2e81b 100644 --- a/src/JsonMapper/Value/Strategy/DateTimeValueConversionStrategy.php +++ b/src/JsonMapper/Value/Strategy/DateTimeValueConversionStrategy.php @@ -30,6 +30,9 @@ /** * Converts ISO-8601 strings and timestamps into date/time value objects. + * + * @internal This is not a public extension point. Register conversions through + * {@see \MagicSunday\JsonMapper\Value\TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ final class DateTimeValueConversionStrategy implements ValueConversionStrategyInterface { diff --git a/src/JsonMapper/Value/Strategy/EnumValueConversionStrategy.php b/src/JsonMapper/Value/Strategy/EnumValueConversionStrategy.php index b25456f..2765fb3 100644 --- a/src/JsonMapper/Value/Strategy/EnumValueConversionStrategy.php +++ b/src/JsonMapper/Value/Strategy/EnumValueConversionStrategy.php @@ -29,6 +29,9 @@ /** * Converts scalar JSON values into enum cases. A backed enum is addressed by case value, a pure * enum by case name. + * + * @internal This is not a public extension point. Register conversions through + * {@see \MagicSunday\JsonMapper\Value\TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ final class EnumValueConversionStrategy implements ValueConversionStrategyInterface { diff --git a/src/JsonMapper/Value/Strategy/NullValueConversionStrategy.php b/src/JsonMapper/Value/Strategy/NullValueConversionStrategy.php index a43269e..6015ae9 100644 --- a/src/JsonMapper/Value/Strategy/NullValueConversionStrategy.php +++ b/src/JsonMapper/Value/Strategy/NullValueConversionStrategy.php @@ -17,6 +17,9 @@ /** * Decides what a null payload means for the declared type. + * + * @internal This is not a public extension point. Register conversions through + * {@see \MagicSunday\JsonMapper\Value\TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ final class NullValueConversionStrategy implements ValueConversionStrategyInterface { diff --git a/src/JsonMapper/Value/Strategy/ObjectTypeConversionGuardTrait.php b/src/JsonMapper/Value/Strategy/ObjectTypeConversionGuardTrait.php index a0a17a6..205a132 100644 --- a/src/JsonMapper/Value/Strategy/ObjectTypeConversionGuardTrait.php +++ b/src/JsonMapper/Value/Strategy/ObjectTypeConversionGuardTrait.php @@ -18,6 +18,9 @@ /** * Provides reusable guards for strategies operating on object types. + * + * @internal This is not a public extension point. Register conversions through + * {@see \MagicSunday\JsonMapper\Value\TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ trait ObjectTypeConversionGuardTrait { diff --git a/src/JsonMapper/Value/Strategy/ObjectValueConversionStrategy.php b/src/JsonMapper/Value/Strategy/ObjectValueConversionStrategy.php index cb9a206..c95cc47 100644 --- a/src/JsonMapper/Value/Strategy/ObjectValueConversionStrategy.php +++ b/src/JsonMapper/Value/Strategy/ObjectValueConversionStrategy.php @@ -25,6 +25,9 @@ /** * Converts object values by delegating to the mapper callback. + * + * @internal This is not a public extension point. Register conversions through + * {@see \MagicSunday\JsonMapper\Value\TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ final readonly class ObjectValueConversionStrategy implements ValueConversionStrategyInterface { diff --git a/src/JsonMapper/Value/Strategy/PassthroughValueConversionStrategy.php b/src/JsonMapper/Value/Strategy/PassthroughValueConversionStrategy.php index e7b41e0..22abe5b 100644 --- a/src/JsonMapper/Value/Strategy/PassthroughValueConversionStrategy.php +++ b/src/JsonMapper/Value/Strategy/PassthroughValueConversionStrategy.php @@ -16,6 +16,9 @@ /** * Fallback strategy returning the value unchanged. + * + * @internal This is not a public extension point. Register conversions through + * {@see \MagicSunday\JsonMapper\Value\TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ final class PassthroughValueConversionStrategy implements ValueConversionStrategyInterface { diff --git a/src/JsonMapper/Value/Strategy/UnionValueConversionStrategy.php b/src/JsonMapper/Value/Strategy/UnionValueConversionStrategy.php index c5b11e1..60cb35e 100644 --- a/src/JsonMapper/Value/Strategy/UnionValueConversionStrategy.php +++ b/src/JsonMapper/Value/Strategy/UnionValueConversionStrategy.php @@ -26,6 +26,9 @@ * elements being the case that mattered - matched no strategy for a union element type and handed * the raw payload back unconverted, without recording anything. Registering the resolution as a * strategy puts it where every consumer of the converter reaches it. + * + * @internal This is not a public extension point. Register conversions through + * {@see \MagicSunday\JsonMapper\Value\TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ final readonly class UnionValueConversionStrategy implements ValueConversionStrategyInterface { diff --git a/src/JsonMapper/Value/Strategy/ValueConversionStrategyInterface.php b/src/JsonMapper/Value/Strategy/ValueConversionStrategyInterface.php index 4bd3d47..947704e 100644 --- a/src/JsonMapper/Value/Strategy/ValueConversionStrategyInterface.php +++ b/src/JsonMapper/Value/Strategy/ValueConversionStrategyInterface.php @@ -16,6 +16,9 @@ /** * Contract for value conversion strategies. + * + * @internal This is not a public extension point. Register conversions through + * {@see \MagicSunday\JsonMapper\Value\TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ interface ValueConversionStrategyInterface { diff --git a/src/JsonMapper/Value/ValueConverter.php b/src/JsonMapper/Value/ValueConverter.php index d4f3324..77a9c05 100644 --- a/src/JsonMapper/Value/ValueConverter.php +++ b/src/JsonMapper/Value/ValueConverter.php @@ -20,6 +20,9 @@ /** * Converts JSON values according to the registered strategies. + * + * @internal This is not a public extension point. Register conversions through + * {@see TypeHandlerInterface} via JsonMapper::addTypeHandler(). */ final class ValueConverter { diff --git a/tests/JsonMapper/Resolver/ClassResolverAllowlistTest.php b/tests/JsonMapper/Resolver/ClassResolverAllowlistTest.php index 1ab8f0e..7e84a80 100644 --- a/tests/JsonMapper/Resolver/ClassResolverAllowlistTest.php +++ b/tests/JsonMapper/Resolver/ClassResolverAllowlistTest.php @@ -290,4 +290,18 @@ public function itRejectsAnAllowlistNamingAClassThatDoesNotExist(): void $resolver->add(Person::class, static fn (): string => VipPerson::class, ['NotAClass']); } + + #[Test] + public function itAcceptsAPlainClassStringTarget(): void + { + // The constructor $classMap already accepts a static SdkFoo => Foo mapping; add() now does + // too, rather than forcing a trivial closure wrapper. resolve() has always handled both. + $resolver = new ClassResolver(); + $resolver->add(Person::class, VipPerson::class); + + self::assertSame( + VipPerson::class, + $resolver->resolve(Person::class, [], new MappingContext([])), + ); + } } diff --git a/tests/JsonMapper/StaticClassMapEntryTest.php b/tests/JsonMapper/StaticClassMapEntryTest.php new file mode 100644 index 0000000..6d2ff65 --- /dev/null +++ b/tests/JsonMapper/StaticClassMapEntryTest.php @@ -0,0 +1,40 @@ + Foo mapping; addCustomClassMapEntry() now + * accepts one too, so a static mapping can be registered at runtime without wrapping it in a + * trivial closure. This drives that through the PUBLIC entry point - the widened ClassResolver::add() + * would otherwise be reachable only from a unit test. + * + * @internal + */ +final class StaticClassMapEntryTest extends TestCase +{ + #[Test] + public function itMapsThroughAStaticClassStringEntry(): void + { + $result = $this->getJsonMapper() + ->addCustomClassMapEntry(Person::class, VipPerson::class) + ->map($this->getJsonAsObject('{"name": "a", "oscars": 3}'), Person::class); + + self::assertInstanceOf(VipPerson::class, $result, 'The static target class is instantiated.'); + self::assertSame('a', $result->name); + self::assertSame(3, $result->oscars); + } +}