Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
boesing opened this issue Aug 8, 2023 · 3 comments
Open

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

boesing opened this issue Aug 8, 2023 · 3 comments
Labels

Comments

@boesing
Copy link
Member

boesing commented Aug 8, 2023

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)

public function extract(object $object): array
{
$result = [];
foreach (self::getReflProperties($object) as $property) {
$propertyName = $this->extractName($property->getName(), $object);
if (! $this->getCompositeFilter()->filter($propertyName)) {
continue;
}
$value = $property->getValue($object);
$result[$propertyName] = $this->extractValue($propertyName, $value, $object);
}
return $result;
}

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 boesing added the RFC label Aug 8, 2023
@boesing
Copy link
Member Author

boesing commented Aug 8, 2023

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.

@boesing boesing changed the title [RFC]: ReflectionHydrator and initialized handling [RFC]: ReflectionHydrator and (un-)initialized property handling Aug 18, 2023
@reinfi
Copy link

reinfi commented Oct 27, 2023

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
Copy link

ezkimo commented May 7, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants