-
-
Notifications
You must be signed in to change notification settings - Fork 514
Change namespace of attribute classes from Annotations to Attribute
#2933
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
Conversation
|
This will force users to migrate without offering anything in return, except for consistency in the namespace with other projects (and not yet ORM). |
|
In fact, ORM don't have this subnamespace (example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR modernizes the MongoDB ODM library's attribute system by moving attribute classes from the Annotations namespace to the new Attribute namespace. The old Annotations namespace is maintained as a deprecated alias to ensure backward compatibility while guiding users toward the new naming convention.
Key Changes:
- Introduced new
Doctrine\ODM\MongoDB\Mapping\Attributenamespace for all attribute classes - Added deprecation triggers when old
Annotationsnamespace classes are loaded - Updated all test files and internal references to use the new namespace
- Maintained backward compatibility through class aliases
Reviewed changes
Copilot reviewed 300 out of 434 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
src/Mapping/Attribute/*.php |
New attribute classes in the Attribute namespace with class aliases to Annotations namespace for backward compatibility |
src/Mapping/Annotations/*.php |
Converted to deprecation stubs that trigger warnings and load the new Attribute classes |
tests/**/*.php |
Updated import statements from Annotations to Attribute namespace |
src/Mapping/Driver/*.php |
Updated internal references to use new Attribute namespace |
src/Mapping/ClassMetadata.php |
Updated type hints to reference Attribute namespace |
UPGRADE-2.16.md |
Added migration guide for the namespace change |
phpstan-baseline.neon |
Updated static analysis baseline with new namespace references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
71eff05 to
5e39484
Compare
malarzm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't quite grasp it, but there's a "class_alias was problematic in the past" dangling in my head. Saying it out loud in case I'd jog somebody's memory with it. Alernative would be to:
- Move classes to Attributes (and drop final, leave it as a phpdoc)
- Classes in Annotations would extend their counterpart, emit a deprecation in ctor and forward everything
- Remove Annotations/* in 3.0 and bring back
finalin Attribute/*
Also I'd like to see docs updated for 2.16 - folks copying documentation should not be greeted by a deprecation :)
@malarzm I wrote an article about that, and @alcaeus wrote a medium article about the aftermath of doing this for |
|
@greg0ire ha! Thanks for the links, I tried to look in our blog and google but prolly I was too specific with "orm namespace change" :) |
5e39484 to
4ae6e4d
Compare
fd80856 to
90f1565
Compare
|
I updated the PR, including the docs to use inheritance. That way the new classes in the New PR description:
I've kept the tests unchanged, ensuring full BC of existing code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 125 out of 125 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
90f1565 to
7c7544d
Compare
alcaeus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how to proceed with the missing Annotation interface on the attribute class aliases. Found one issue where annotation classes were used in the attribute reader.
| } | ||
|
|
||
| // @phpstan-ignore class.notFound | ||
| class_alias(AbstractDocument::class, \Doctrine\ODM\MongoDB\Mapping\Annotations\AbstractDocument::class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted that this works in conjunction with the class_exists call in the AbstractDocument annotation file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exact, having the class_alias in the same file as the class declaration ensure we always have both declared, even if the attribute is used first.
And the class_exists only triggers the autoloader.
| use function array_any; | ||
| use function array_find; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the use of array_any and array_find in this class 👏
| // Make sure we only get Doctrine Annotations | ||
| if (! is_subclass_of($attributeName, Annotation::class)) { | ||
| // Make sure we only get MongoDB ODM attribute classes | ||
| if (! is_subclass_of($attributeName, MappingAttribute::class)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted that this doesn't change existing behaviour, as all annotations extend the attribute classes.
fcb30bc to
7a756e1
Compare
GromNaN
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how to proceed with the missing Annotation interface on the attribute class aliases. Found one issue where annotation classes were used in the attribute reader.
Do you have an example, I don't see the issue.
This classes are never loaded
The Attribute classes cannot be used as Annotation Move QueryType enum to the Mapping namespace Keep the tests on the Annotations namespace
7a756e1 to
416ff17
Compare
416ff17 to
0e9bf0d
Compare
|
Before merging, we need to update the bundle: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 125 out of 125 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Doctrine\ODM\MongoDB\Mapping\AnnotationstoDoctrine\ODM\MongoDB\Mapping\Attribute. Removing the@Annotationso that they can't be used for legacy annotations.Annotationsnamespace, they extend those from theAttributenamespace. Adding the legacy@Annotationfor BC.EncryptQueryenum is moved to the parentDoctrine\ODM\MongoDB\MappingnamespaceRelated initiative in Symfony: symfony/symfony#51381