From e60f0d3bc73daa24b7e05495dce4e34398a9947e Mon Sep 17 00:00:00 2001 From: Alex Wells Date: Mon, 10 Mar 2025 15:27:18 +0200 Subject: [PATCH 1/3] feat: A callback for reporting unexpected enum values --- README.md | 65 ++++++++++++++++--- src/SerializerBuilder.php | 22 ++++++- .../Property/DefaultBoundClassProperty.php | 10 ++- .../DefaultBoundClassPropertyFactory.php | 10 ++- tests/Integration/JsonSerializationTest.php | 13 ++++ tests/Stubs/UseDefaultStub.php | 15 +++++ 6 files changed, 120 insertions(+), 15 deletions(-) create mode 100644 tests/Stubs/UseDefaultStub.php diff --git a/README.md b/README.md index 7b10f41..b5f4e34 100644 --- a/README.md +++ b/README.md @@ -137,9 +137,9 @@ to use in order of priority: ```php (new SerializerBuilder()) - ->addMapperLast(new TestMapper()) // then this one - ->addFactoryLast(new TestFactory()) // and this one last - ->addFactory(new TestFactory()) // attempted first + ->addMapperLast(new TestMapper()) // #2 - then this one + ->addFactoryLast(new TestFactory()) // #3 - and this one last + ->addFactory(new TestFactory()) // #1 - attempted first ``` A factory has the following signature: @@ -205,11 +205,11 @@ methods: `serialize` and `deserialize`. They do exactly what they're called. ## Naming of keys -By default serializer preserves the naming of keys, but this is easily customizable (in order of priority): +By default, serializer preserves the naming of keys, but this is easily customizable (in order of priority): - specify a custom property name using the `#[SerializedName]` attribute - specify a custom naming strategy per class using the `#[SerializedName]` attribute -- specify a custom global naming strategy (use one of the built in or write your own) +- specify a custom global (default) naming strategy (use one of the built-in or write your own) Here's an example: @@ -235,7 +235,7 @@ class Item2 { ``` Out of the box, strategies for `snake_case`, `camelCase` and `PascalCase` are provided, -but you it's trivial to implement your own: +but it's trivial to implement your own: ```php class PrefixedNaming implements NamingStrategy { @@ -255,7 +255,7 @@ class SiftTrackData {} ## Required, nullable, optional and default values -By default if a property is missing in serialized payload: +By default, if a property is missing in serialized payload: - nullable properties are just set to null - properties with a default value - use the default value @@ -319,6 +319,51 @@ $adapter->serialize( ); ``` +## Use default value for unexpected + +There are situations where you're deserializing data from a third party that doesn't have an API documentation +or one that can't keep a backwards compatibility promise. One such case is when a third party uses an enum +and you expect that new enum values might get added in the future by them. For example, imagine this structure: + +```php +enum CardType: string +{ + case CLUBS = 'clubs'; + case DIAMONDS = 'diamonds'; + case HEARTS = 'hearts'; + case SPADES = 'spades'; +} + +readonly class Card { + public function __construct( + public CardType $type, + public string $value, + ) {} +} +``` + +If you get an unexpected value for `type`, you'll get an exception: + +```php +// UnexpectedEnumValueException: Expected one of [clubs, diamonds, hearts, spades], but got 'joker' +$adapter->deserialize('{"type": "joker"}'); +``` + +So if you suspect that might happen, add a default value you wish to use (anything) and +a `#[UseDefaultForUnexpected]` attribute: + +```php +readonly class Card { + public function __construct( + #[UseDefaultForUnexpected] + public CardType $type = null, + // Can be any other valid default value + #[UseDefaultForUnexpected] + public CardType $type2 = CardType::SPADES, + ) {} +} +``` + ## Error handling This is expected to be used with client-provided data, so good error descriptions is a must. @@ -369,9 +414,9 @@ There are some alternatives to this, but all of them will lack at least one of t - doesn't rely on inheritance, hence allows serializing third-party classes - parses existing PHPDoc information instead of duplicating it through attributes -- supports generic types which are extremely useful for wrapper types +- supports generic types which are quite useful for wrapper types - allows simple extension through mappers and complex stuff through type adapters - produces developer-friendly error messages for invalid data -- correctly handles optional (missing keys) and `null` values as separate concepts +- correctly handles optional (missing keys) and `null` values as separate concerns - simple to extend with additional formats -- simple internal structure: no node tree, no value wrappers, no PHP parsing, no inherent limitations +- simple internal structure: no node tree, no value/JSON wrappers, no custom reflection / PHP parsing, no inherent limitations diff --git a/src/SerializerBuilder.php b/src/SerializerBuilder.php index 84e94d0..0f82034 100644 --- a/src/SerializerBuilder.php +++ b/src/SerializerBuilder.php @@ -9,6 +9,7 @@ use GoodPhp\Serialization\Serializer\Registry\Cache\MemoizingTypeAdapterRegistry; use GoodPhp\Serialization\Serializer\Registry\Factory\FactoryTypeAdapterRegistryBuilder; use GoodPhp\Serialization\Serializer\TypeAdapterRegistrySerializer; +use GoodPhp\Serialization\TypeAdapter\Exception\UnexpectedValueException; use GoodPhp\Serialization\TypeAdapter\Json\FromPrimitiveJsonTypeAdapterFactory; use GoodPhp\Serialization\TypeAdapter\Primitive\BuiltIn\ArrayMapper; use GoodPhp\Serialization\TypeAdapter\Primitive\BuiltIn\BackedEnumMapper; @@ -19,6 +20,7 @@ use GoodPhp\Serialization\TypeAdapter\Primitive\ClassProperties\Naming\BuiltInNamingStrategy; use GoodPhp\Serialization\TypeAdapter\Primitive\ClassProperties\Naming\NamingStrategy; use GoodPhp\Serialization\TypeAdapter\Primitive\ClassProperties\Naming\SerializedNameAttributeNamingStrategy; +use GoodPhp\Serialization\TypeAdapter\Primitive\ClassProperties\Property\BoundClassProperty; use GoodPhp\Serialization\TypeAdapter\Primitive\ClassProperties\Property\BoundClassPropertyFactory; use GoodPhp\Serialization\TypeAdapter\Primitive\ClassProperties\Property\DefaultBoundClassPropertyFactory; use GoodPhp\Serialization\TypeAdapter\Primitive\Illuminate\CollectionMapper; @@ -44,6 +46,9 @@ final class SerializerBuilder private ?MapperMethodFactory $mapperMethodFactory = null; + /** @var callable(BoundClassProperty, UnexpectedValueException): void|null */ + private mixed $reportUnexpectedDefault = null; + public function withReflector(Reflector $reflector): self { Assert::null($this->reflector, 'You must set the reflector before adding any mappers or factories.'); @@ -126,6 +131,19 @@ public function addMapperLast(object $adapter): self return $that; } + /** + * Define a callback for cases where a default value is substituted when using #[UseDefaultForUnexpected] + * + * @param callable(BoundClassProperty, UnexpectedValueException): void|null $callback + */ + public function reportUnexpectedDefault(?callable $callback): self + { + $that = clone $this; + $that->reportUnexpectedDefault = $callback; + + return $that; + } + public function build(): Serializer { $typeAdapterRegistryBuilder = $this->typeAdapterRegistryBuilder() @@ -139,7 +157,9 @@ public function build(): Serializer ->addFactoryLast(new ClassPropertiesPrimitiveTypeAdapterFactory( new SerializedNameAttributeNamingStrategy($this->namingStrategy ?? BuiltInNamingStrategy::PRESERVING), $this->hydrator ?? new ConstructorHydrator(), - $this->boundClassPropertyFactory ?? new DefaultBoundClassPropertyFactory(), + $this->boundClassPropertyFactory ?? new DefaultBoundClassPropertyFactory( + $this->reportUnexpectedDefault, + ), )) ->addFactoryLast(new FromPrimitiveJsonTypeAdapterFactory()); diff --git a/src/TypeAdapter/Primitive/ClassProperties/Property/DefaultBoundClassProperty.php b/src/TypeAdapter/Primitive/ClassProperties/Property/DefaultBoundClassProperty.php index d5570e4..c8ae235 100644 --- a/src/TypeAdapter/Primitive/ClassProperties/Property/DefaultBoundClassProperty.php +++ b/src/TypeAdapter/Primitive/ClassProperties/Property/DefaultBoundClassProperty.php @@ -26,8 +26,9 @@ final class DefaultBoundClassProperty implements BoundClassProperty { /** - * @param PropertyReflection> $property - * @param TypeAdapter $typeAdapter + * @param PropertyReflection> $property + * @param TypeAdapter $typeAdapter + * @param callable(BoundClassProperty, UnexpectedValueException): void|null $reportUnexpectedDefault */ public function __construct( private readonly PropertyReflection $property, @@ -37,6 +38,7 @@ public function __construct( private readonly bool $hasDefaultValue, private readonly bool $nullable, private readonly bool $useDefaultForUnexpected, + private readonly mixed $reportUnexpectedDefault = null, ) { if ($this->useDefaultForUnexpected) { Assert::true($this->hasDefaultValue, 'When using #[UseDefaultForUnexpected], the property must have a default value.'); @@ -92,6 +94,10 @@ public function deserialize(array $data): array ]; } catch (UnexpectedValueException $e) { if ($this->useDefaultForUnexpected) { + if ($this->reportUnexpectedDefault !== null) { + ($this->reportUnexpectedDefault)($this, $e); + } + return []; } diff --git a/src/TypeAdapter/Primitive/ClassProperties/Property/DefaultBoundClassPropertyFactory.php b/src/TypeAdapter/Primitive/ClassProperties/Property/DefaultBoundClassPropertyFactory.php index 09794ea..e84428a 100644 --- a/src/TypeAdapter/Primitive/ClassProperties/Property/DefaultBoundClassPropertyFactory.php +++ b/src/TypeAdapter/Primitive/ClassProperties/Property/DefaultBoundClassPropertyFactory.php @@ -10,6 +10,7 @@ use GoodPhp\Reflection\Type\Type; use GoodPhp\Serialization\MissingValue; use GoodPhp\Serialization\Serializer; +use GoodPhp\Serialization\TypeAdapter\Exception\UnexpectedValueException; use GoodPhp\Serialization\TypeAdapter\Primitive\ClassProperties\Property\Flattening\Flatten; use GoodPhp\Serialization\TypeAdapter\Primitive\ClassProperties\Property\Flattening\FlatteningBoundClassProperty; use Webmozart\Assert\Assert; @@ -18,8 +19,12 @@ class DefaultBoundClassPropertyFactory implements BoundClassPropertyFactory { private readonly NamedType $missingValueType; - public function __construct() - { + /** + * @param callable(BoundClassProperty, UnexpectedValueException): void|null $reportUnexpectedDefault + */ + public function __construct( + private readonly mixed $reportUnexpectedDefault = null, + ) { $this->missingValueType = new NamedType(MissingValue::class); } @@ -49,6 +54,7 @@ public function create( hasDefaultValue: $this->hasDefaultValue($property), nullable: $type instanceof NullableType, useDefaultForUnexpected: $property->attributes()->has(UseDefaultForUnexpected::class), + reportUnexpectedDefault: $this->reportUnexpectedDefault, ); } diff --git a/tests/Integration/JsonSerializationTest.php b/tests/Integration/JsonSerializationTest.php index cfe52ec..691299b 100644 --- a/tests/Integration/JsonSerializationTest.php +++ b/tests/Integration/JsonSerializationTest.php @@ -23,6 +23,7 @@ use Tests\Stubs\BackedEnumStub; use Tests\Stubs\ClassStub; use Tests\Stubs\NestedStub; +use Tests\Stubs\UseDefaultStub; use Tests\Stubs\ValueEnumStub; use Throwable; @@ -335,6 +336,18 @@ public static function deserializesProvider(): iterable ), '{"primitive":1,"nested":{},"date":"2020-01-01T00:00:00.000000Z","carbonImmutable":"2020-01-01T00:00:00.000000Z"}', ]; + + yield '#[UseDefaultForUnexpected] with unexpected values' => [ + new NamedType(UseDefaultStub::class), + new UseDefaultStub(), + '{"null":"unknown value","enum":"also unknown"}', + ]; + + yield '#[UseDefaultForUnexpected] with expected values' => [ + new NamedType(UseDefaultStub::class), + new UseDefaultStub(BackedEnumStub::ONE, BackedEnumStub::TWO), + '{"null":"one","enum":"two"}', + ]; } /** diff --git a/tests/Stubs/UseDefaultStub.php b/tests/Stubs/UseDefaultStub.php new file mode 100644 index 0000000..e642c73 --- /dev/null +++ b/tests/Stubs/UseDefaultStub.php @@ -0,0 +1,15 @@ + Date: Mon, 10 Mar 2025 15:32:17 +0200 Subject: [PATCH 2/3] fix: Failing tests --- .../Registry/Cache/MemoizingTypeAdapterRegistry.php | 4 ++-- .../Registry/Factory/FactoryTypeAdapterRegistry.php | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Serializer/Registry/Cache/MemoizingTypeAdapterRegistry.php b/src/Serializer/Registry/Cache/MemoizingTypeAdapterRegistry.php index b71677d..ce6a988 100644 --- a/src/Serializer/Registry/Cache/MemoizingTypeAdapterRegistry.php +++ b/src/Serializer/Registry/Cache/MemoizingTypeAdapterRegistry.php @@ -14,7 +14,7 @@ final class MemoizingTypeAdapterRegistry implements TypeAdapterRegistry { - /** @var array }>>>> */ + /** @var array }>>>> */ private array $resolved = []; public function __construct( @@ -35,7 +35,7 @@ public function forType(string $typeAdapterType, Serializer $serializer, Type $t $matchingFactories = $this->resolved[$typeAdapterType][(string) $type][$skipPast ?? $serializer] ??= []; - /** @var array{ object[], TypeAdapter }|null $attributesFactoryPair */ + /** @var array{ Attributes, TypeAdapter }|null $attributesFactoryPair */ $attributesFactoryPair = Arr::first($matchingFactories, fn (array $pair) => $attributes->allEqual($pair[0])); if ($attributesFactoryPair) { diff --git a/src/Serializer/Registry/Factory/FactoryTypeAdapterRegistry.php b/src/Serializer/Registry/Factory/FactoryTypeAdapterRegistry.php index 470b41a..593b787 100644 --- a/src/Serializer/Registry/Factory/FactoryTypeAdapterRegistry.php +++ b/src/Serializer/Registry/Factory/FactoryTypeAdapterRegistry.php @@ -50,6 +50,7 @@ public function forType(string $typeAdapterType, Serializer $serializer, Type $t $factory = $this->factories[$i]; if ($adapter = $factory->create($typeAdapterType, $type, $attributes, $serializer)) { + /* @phpstan-ignore-next-line */ Assert::isInstanceOf($adapter, $typeAdapterType); /** @var TypeAdapterType */ From cb461e91123c952b53116e17d83137938e55d7f2 Mon Sep 17 00:00:00 2001 From: Alex Wells Date: Mon, 10 Mar 2025 15:38:34 +0200 Subject: [PATCH 3/3] docs: Add docs for reporting unexpected defaults --- README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/README.md b/README.md index b5f4e34..74fa8dd 100644 --- a/README.md +++ b/README.md @@ -364,6 +364,19 @@ readonly class Card { } ``` +Whenever that happens, a default value will be used instead. Optionally, you can also log such cases: + +```php +$serializer = (new SerializerBuilder()) + ->reportUnexpectedDefault(function (BoundClassProperty $property, UnexpectedValueException $e) { + $log->warning("Serializer used a default for unexpected value: {$e->getMessage()}", [ + 'property' => $property->serializedName(), + 'exception' => $e, + ]); + }) + ->build(); +``` + ## Error handling This is expected to be used with client-provided data, so good error descriptions is a must.