laminas / laminas-hydrator

Serialize objects to arrays, and vice versa
https://docs.laminas.dev/laminas-hydrator/
BSD 3-Clause "New" or "Revised" License
123 stars 32 forks source link

[RFC]: `ReflectionHydrator` and (un-)initialized property handling #112

Open boesing opened 1 year ago

boesing commented 1 year ago

RFC

Q A
Proposed Version(s) next minor
BC Break? No

Goal

Skip uninitialized properties in extract method.

Background

Every hydrator needs an instance of an object to be passed to the methods by-design. We are currently moving over from non-typed properties to typed properties + constructor promotion. That works well once the object is instantiated within the code by using the construct but we do also have a scope where we hydrate and extract those objects (don't ask...).

However, at some point, it seems that we do only have a partial set of data to hydrate into the object. Prior typed properties, every property which was not explicitly passed via data argument was initialized as null and thus, there was not a problem (besides the fact that even that a method/property stated to return an object/string/whatever returned null once you wanted to use that value).

So once you extract an object which was not 100% hydrated before leads to an error because the property is not properly initialized.

Considerations

There should be no real impact on users besides the fact that objects with uninitialized properties do behave the same way with and without typed properties (being null in the extracted data).

Proposal(s)

https://github.com/laminas/laminas-hydrator/blob/15cd4da16cd1dcf71651a684fd3d591121a56745/src/ReflectionHydrator.php#L24-L38

We could add a new private method to return null for uninitialized properties:

private function getPropertyValue(ReflectionProperty $property, object $object): mixed
{
      if ($property->hasType() && !$property->isInitialized($object)) {
           return null;
      }

      return $property->getValue($object);
}

Appendix

/

boesing commented 1 year ago

Only solution for me right now (in our project where this error occurs) is to remove the typed properties even tho they would provide better manual instantiation of the object.

Imho we could add this behavior as opt-in if some1 wants the hydrator to fail badly but I leave this open for discussion.

reinfi commented 1 year ago

Would love to have this, because using form hydration I always need to have two objects when using typed properties. One for the update case and one for the add case because the update case has an id/timestamp or something else.

ezkimo commented 6 months ago

Actually I 'm archieving exactly the same problem. I 'm always running into a thrown Error because of uninitialized properties. Since constructor property promotion and readonly properties / classes were introduced, I 'm using it for data objects, because they only exist for holding data. Just define some public properties and forget all those getters and setters.

I 'm using a slightly edited version of the reflection hydrator, which exactly has the same change, as you pointed out in your RFC to set unitialized properties to initialized properties by giving 'em a value of null. An option in form of a flag, that enables / disables this behaviour would be great.