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.

Sign up for GitHub

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

Jump to bottom

ClassMethodsHydrator::extract does not work with anonymous classes #25

Closed
luiz-brandao opened this issue Sep 25, 2020 · 2 comments · Fixed by #30
Closed

ClassMethodsHydrator::extract does not work with anonymous classes #25

luiz-brandao opened this issue Sep 25, 2020 · 2 comments · Fixed by #30
Labels
Bug Something isn't working Feature Request
Milestone
4.0.0

Comments

@luiz-brandao
Copy link

luiz-brandao commented Sep 25, 2020 •
edited

Bug Report

Q A
Version(s) 3.1.0

Summary

Using OptionalParametersFilter with an anonymous class fails saying a method does not exist.

Current behavior

The property FQN generated at ClassMethodsHydrator.php:157 does not work for anonymous classes. For instance, in my case I get something like this:

class@anonymous\000/path/to/file/MyTest.php0x7fabdb08bdf5::getSomeField

Consequently, OptionalParametersFilter.php:48 fails because new ReflectionMethod can't locate it without an object.

$reflectionMethod = new ReflectionMethod($property);

How to reproduce

See unit test:

#26

Expected behavior

Values to be properly extracted

Possible solutions

It seems we can't extract using the FQN but instead we need to use a reference to the object.

$reflectionMethod = new ReflectionMethod($object, $methodName);
@luiz-brandao luiz-brandao mentioned this issue Sep 25, 2020
Closed
@luiz-brandao
Copy link
Author

luiz-brandao commented Sep 25, 2020 •
edited

The issue with providing a fix is that the FilterInterface::filter expects a string and changing that would affect the whole framework.

Locally, as a proof-of-concept, I managed to fix the issue by creating a new OptionalParametersFilter::filterWithoutFqn method. Basically it's a copy of OptionalParametersFilter::filter that accepts an object instance and the method name instead of a property FQN. That also means I had to change ClassMethodsHydrator::extract to call the new method.

Feedback from someone that has more in depth knowledge of this library would be essential.

Thank you.

@weierophinney weierophinney linked a pull request Sep 28, 2020 that will close this issue
Add new test case in ClassMethodsHydratorTest #26
Closed
weierophinney added a commit to weierophinney/laminas-hydrator that referenced this issue Oct 5, 2020
@weierophinney
fix: ensure hydrators work with anonymous objects
2acd9ab 
This patch modifies filters slightly.

Previously, they already checked to see if static property notation was provided or not.
This extends that slightly to allow passing an optional second argument, a `null` or `object` value representing the instance being extracted or hydrated.
For those filters that then use reflection, the `$instance` can be used as the primary argument, and the `$property` argument as the second.
This approach allows the `ClassMethodsHydrator` to filter properly based on the specific instance under scrutiny.

Additionally, this makes changes to `ClassMethodsHydrator` to:

- Detect if an anonymous class was used.
- Use the `spl_object_hash()` of that instance for caching the method list.
- Pass the `$object` as the `$instance` argument to filters.

Finally, I've added tests to each of the hydrators to verify they work as expected when presented with anonymous objects.

Fixes laminas#25
Fixes laminas#26

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
@weierophinney weierophinney mentioned this issue Oct 5, 2020
Merged
weierophinney added a commit to weierophinney/laminas-hydrator that referenced this issue Oct 5, 2020
@weierophinney
fix: ensure hydrators work with anonymous objects
631df2e 
This patch modifies filters slightly.

Previously, they already checked to see if static property notation was provided or not.
This extends that slightly to allow passing an optional second argument, a `null` or `object` value representing the instance being extracted or hydrated.
For those filters that then use reflection, the `$instance` can be used as the primary argument, and the `$property` argument as the second.
This approach allows the `ClassMethodsHydrator` to filter properly based on the specific instance under scrutiny.

Additionally, this makes changes to `ClassMethodsHydrator` to:

- Detect if an anonymous class was used.
- Use the `spl_object_hash()` of that instance for caching the method list.
- Pass the `$object` as the `$instance` argument to filters.

Finally, I've added tests to each of the hydrators to verify they work as expected when presented with anonymous objects.

Fixes laminas#25
Fixes laminas#26

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
@weierophinney weierophinney added this to the 3.2.0 milestone Oct 5, 2020
@weierophinney weierophinney added Bug Something isn't working Feature Request labels Oct 5, 2020
@weierophinney
Copy link
Member

Honestly, this all predates anonymous classes by a number of years, so I'm not surprised it doesn't work.

I've labelled it as both a bug and a feature (bug, in that it should work; feature, in that it's technically new functionality), and built PR #30 off of your PR #26 to address it.

weierophinney added a commit to weierophinney/laminas-hydrator that referenced this issue Oct 6, 2020
@weierophinney
fix: ensure hydrators work with anonymous objects
4972655 
This patch modifies filters slightly.

Previously, they already checked to see if static property notation was provided or not.
This extends that slightly to allow passing an optional second argument, a `null` or `object` value representing the instance being extracted or hydrated.
For those filters that then use reflection, the `$instance` can be used as the primary argument, and the `$property` argument as the second.
This approach allows the `ClassMethodsHydrator` to filter properly based on the specific instance under scrutiny.

Additionally, this makes changes to `ClassMethodsHydrator` to:

- Detect if an anonymous class was used.
- Use the `spl_object_hash()` of that instance for caching the method list.
- Pass the `$object` as the `$instance` argument to filters.

Finally, I've added tests to each of the hydrators to verify they work as expected when presented with anonymous objects.

Fixes laminas#25
Fixes laminas#26

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
weierophinney added a commit to weierophinney/laminas-hydrator that referenced this issue Oct 6, 2020
@weierophinney
fix: ensure hydrators work with anonymous objects
10b62a1 
This patch modifies filters slightly.

Previously, they already checked to see if static property notation was provided or not.
This extends that slightly to allow passing an optional second argument, a `null` or `object` value representing the instance being extracted or hydrated.
For those filters that then use reflection, the `$instance` can be used as the primary argument, and the `$property` argument as the second.
This approach allows the `ClassMethodsHydrator` to filter properly based on the specific instance under scrutiny.

Additionally, this makes changes to `ClassMethodsHydrator` to:

- Detect if an anonymous class was used.
- Use the `spl_object_hash()` of that instance for caching the method list.
- Pass the `$object` as the `$instance` argument to filters.

Finally, I've added tests to each of the hydrators to verify they work as expected when presented with anonymous objects.

Fixes laminas#25
Fixes laminas#26

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
@weierophinney weierophinney modified the milestones: 3.2.0, 4.0.0 Oct 6, 2020
weierophinney added a commit to weierophinney/laminas-hydrator that referenced this issue Oct 6, 2020
@weierophinney
fix: ensure hydrators work with anonymous objects
38d3f15 
This patch modifies filters slightly.

Previously, they already checked to see if static property notation was provided or not.
This extends that slightly to allow passing an optional second argument, a `null` or `object` value representing the instance being extracted or hydrated.
For those filters that then use reflection, the `$instance` can be used as the primary argument, and the `$property` argument as the second.
This approach allows the `ClassMethodsHydrator` to filter properly based on the specific instance under scrutiny.

Additionally, this makes changes to `ClassMethodsHydrator` to:

- Detect if an anonymous class was used.
- Use the `spl_object_hash()` of that instance for caching the method list.
- Pass the `$object` as the `$instance` argument to filters.

Finally, I've added tests to each of the hydrators to verify they work as expected when presented with anonymous objects.

Fixes laminas#25
Fixes laminas#26

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Something isn't working Feature Request
Projects
None yet
Milestone
4.0.0
2 participants
@weierophinney @luiz-brandao