search

EventSaucePHP / ObjectHydrator

Object Hydration library to create Command and Query objects.
MIT License
318 stars 24 forks source link

Can't pass-through array without further hydration, even with custom caster #56

Closed bradjones1 closed 1 year ago

bradjones1 commented 1 year ago

I am using ObjectHydrator as a way to both do some basic sanity-checking on responses from third-party APIs (by way of hydration) and then using that value object to pass around.

In some cases I wish to cast complex data properties into a further value-object classes, but in other cases I want to preserve an array "as-is" without being cast. This works fine for a "reflection-only" approach where the type of the property in the constructor is simply array.

However, if you also have a docblock (as my code standard mandates) that simply says array for this same property, you raise an exception Unable to hydrate object: The\\Class\nCaused by: Unable to resolve item type for type: array

This is due to the code in \EventSauce\ObjectHydrator\NaivePropertyTypeResolver::extractItemType which assumes a docblock will always specify a type for all the members of the array in either phpdoc or phpstan format.

The workaround appears to be to use the phpstan style array<mixed> type annotation (mixed if you want maximum compatibility) but this is definitely a WTF.

I think perhaps we could add some extra special-casing to ::extractItemType to return array if $type === 'array'? Was this a design choice, though? In that case this is a documentation item?

bradjones1 commented 1 year ago

I suppose we could also add additional language in the exception to hint you in the right direction, like "If you intend to retain an array without casting or explicitly specifying its members, change the docblock to array<int|integer|string|mixed>"

bradjones1 commented 1 year ago

Went down this rabbit hole and determined that the docblock test coverage is still lacking (e.g., in addition to the recently fixed https://github.com/EventSaucePHP/ObjectHydrator/pull/50) and also that we can better support simple array docblock param types by inferring that as containing mixed members, and also allowing various variations of FQCNs in docblocks as well. Looks like no regressions in the rest of the test suite (at least from my local testing) so hopefully these changes can be confidently merged.

frankdejonge commented 1 year ago

Hi @bradjones1 thanks for digging into this! A+ grade contribution :D I'll make sure to release the related PR today.

bradjones1 commented 1 year ago

@frankdejonge Thanks for the prompt review and the compliment, it means a lot coming from you!

bradjones1 commented 1 year ago

Could we also publish a release? Refs https://github.com/EventSaucePHP/ObjectHydrator/issues/49