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

Accept both ClassMetadata and ClassMetadataInfo objects from the ORM #2716

Merged
merged 1 commit into from
Nov 25, 2023

Conversation

mbabker
Copy link
Contributor

@mbabker mbabker commented Oct 29, 2023

Ref: #2708

The Doctrine\ORM\Mapping\ClassMetadataInfo class is deprecated in ORM 2.x and removed entirely in 3.0, with Doctrine\ORM\Mapping\ClassMetadata being the replacement. Doctrine\ORM\Mapping\ClassMetadata exists in 2.x as a subclass of Doctrine\ORM\Mapping\ClassMetadataInfo, so no version conditional behaviors are really needed here, therefore, this PR just adds both ORM classes to some existing doc block types and assertions (I'm aware there are other places using the deprecated ClassMetadataInfo class but the changes here just unblocked being able to run a number of tests locally on ORM 3.0 for now).

@codecov
Copy link

codecov bot commented Oct 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (058e95a) 79.36% compared to head (fea79c7) 79.36%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2716   +/-   ##
=======================================
  Coverage   79.36%   79.36%           
=======================================
  Files         162      162           
  Lines        8457     8457           
=======================================
  Hits         6712     6712           
  Misses       1745     1745           
Files Coverage Δ
src/Mapping/ExtensionMetadataFactory.php 83.90% <ø> (ø)
src/Mapping/MappedEventSubscriber.php 87.20% <ø> (ø)

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

Copy link
Collaborator

@franmomu franmomu left a comment

Choose a reason for hiding this comment

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

I guess we are keeping ClassMetadataInfo in case someone is extending it (unlikely, but 🤷‍♂️ ).

@franmomu franmomu requested a review from phansys October 30, 2023 22:22
@@ -229,7 +230,7 @@ final public function setCacheItemPool(CacheItemPoolInterface $cacheItemPool): v
*/
public function loadMetadataForObjectClass(ObjectManager $objectManager, $metadata)
{
assert($metadata instanceof DocumentClassMetadata || $metadata instanceof EntityClassMetadata);
assert($metadata instanceof DocumentClassMetadata || $metadata instanceof EntityClassMetadata || $metadata instanceof LegacyEntityClassMetadata);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we could add the dockblock @phpstan-assert ClassMetadata&(DocumentClassMetadata|EntityClassMetadata|LegacyEntityClassMetadata) $meta.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried it, that assertion didn't change anything on its own. Removing the assert() runtime call added errors, or trying @param ClassMetadata&(DocumentClassMetadata|EntityClassMetadata|LegacyEntityClassMetadata) $metadata like is done elsewhere added more errors.

@franmomu franmomu merged commit 5d0b93d into doctrine-extensions:main Nov 25, 2023
19 checks passed
@franmomu
Copy link
Collaborator

thanks @mbabker!

@mbabker mbabker deleted the orm-3-compat-metadata branch November 25, 2023 22:42
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.

3 participants