Skip to content

Replace ClassMetadata::getReflectionProperty with (get|set)FieldValue#3033

Open
Jean85 wants to merge 2 commits intodoctrine-extensions:mainfrom
Jean85:resolve-reflection-deprecation
Open

Replace ClassMetadata::getReflectionProperty with (get|set)FieldValue#3033
Jean85 wants to merge 2 commits intodoctrine-extensions:mainfrom
Jean85:resolve-reflection-deprecation

Conversation

@Jean85
Copy link
Contributor

@Jean85 Jean85 commented Jan 28, 2026

This avoids a new deprecation.

Fixes #3032

$uow = $om->getUnitOfWork();
foreach ($config['encode'] as $field => $options) {
$value = $meta->getReflectionProperty($field)->getValue($object);
$value = Meta::getProperty($meta, $field)->getValue($object);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test failures around this line mean if you're going to use a static helper (which really needs to be flagged @internal), it needs to support the persistence ClassMetadata interface and not a single implementation of it. There are a lot of code paths which are supporting MongoDB ODM and ORM (this one included, see the Gedmo\Tests\Mapping\ExtensionODMTest::testGeneratedValues failure on PHP 8.5) so a bulk search/replace isn't feasible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roger. I'm pushing an alternative approach, it should be enough because it relies on direct method existence checks; unfortunately the involved methods are not present in the interface.

@Jean85
Copy link
Contributor Author

Jean85 commented Jan 29, 2026

@mbabker I've changed my approach as suggested, but I've discovered some additional stuff:

  • all code paths that I changed are in tests -- should I move the helper to the test folder?
  • I opened this PR because I thought I needed to solve some deprecations being triggered on my project, while all is already solved in 3ba0096, I just need to update... I can still complete this PR if you want

@mbabker
Copy link
Contributor

mbabker commented Jan 29, 2026

  • I opened this PR because I thought I needed to solve some deprecations being triggered on my project, while all is already solved in 3ba0096, I just need to update... I can still complete this PR if you want

At this point I'd just change it similar to that commit/PR. After doctrine/persistence#451 getFieldValue() and setFieldValue() are part of the metadata interface and not just something provided by every implementation, so there's no need for consumers to know if they need a reflection property or a property accessor (that's all handled by the metadata).

@Jean85 Jean85 force-pushed the resolve-reflection-deprecation branch from d0916b0 to 3b3010c Compare January 30, 2026 23:06
@Jean85 Jean85 changed the title Replace ClassMetadata::getReflectionProperty with getPropertyAccessor Replace ClassMetadata::getReflectionProperty with (get|set)FieldValue Jan 30, 2026
@Jean85 Jean85 requested a review from mbabker January 30, 2026 23:08
@Jean85
Copy link
Contributor Author

Jean85 commented Jan 30, 2026

@mbabker I did it; the two failing tests are unrelated, I see them failing on main too, locally at least.

@Jean85 Jean85 force-pushed the resolve-reflection-deprecation branch from 9d7b9ee to f0960ac Compare January 31, 2026 17:57
@Jean85
Copy link
Contributor Author

Jean85 commented Jan 31, 2026

@mbabker now the build is green; I had to fix it with f0960ac, let me know if that's ok.

@codecov
Copy link

codecov bot commented Jan 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.82%. Comparing base (eef8a13) to head (f0960ac).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3033      +/-   ##
==========================================
+ Coverage   78.75%   78.82%   +0.06%     
==========================================
  Files         169      169              
  Lines        8695     8688       -7     
==========================================
  Hits         6848     6848              
+ Misses       1847     1840       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ReflectionProperty is deprecated in ORM 3.5+

2 participants