doctrine/archived-orm
1
0
Fork 0
mirror of https://github.com/doctrine/orm.git synced 2026-03-24 06:52:09 +01:00
Code Issues 5.3k Packages Projects Releases 100 Wiki Activity

Annotation receives array of paths instead of reader #6963

New Issue
Closed
opened 2026-01-22 15:42:13 +01:00 by admin · 21 comments
admin commented 2026-01-22 15:42:13 +01:00
Owner
Copy Link

Originally created by @ppaulis on GitHub (Apr 20, 2022).

Bug Report

2.12.0 doesn't work with doctrine/doctrine-orm-module : 5.1.0.

Q A
BC Break yes?
Version 2.12.0

Summary

After upgrading from 2.11.1 to 2.12.0, I'm experiencing the following issue:

/var/www/html # vendor/bin/doctrine-module orm:generate-proxies
PHP Fatal error:  Uncaught Error: Call to a member function getClassAnnotations() on array in /var/www/html/vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php:82
Stack trace:
#0 /var/www/html/vendor/doctrine/persistence/src/Persistence/Mapping/Driver/MappingDriverChain.php(79): Doctrine\ORM\Mapping\Driver\AnnotationDriver->loadMetadataForClass('tms\\V1\\Rest\\Sub...', Object(Doctrine\ORM\Mapping\ClassMetadata))
#1 /var/www/html/vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php(133): Doctrine\Persistence\Mapping\Driver\MappingDriverChain->loadMetadataForClass('tms\\V1\\Rest\\Sub...', Object(Doctrine\ORM\Mapping\ClassMetadata))

Current behavior

Reading Metadata no longer works through the doctrine-orm-module.

In the following line:
d550364431/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php (L82)
$this->reader no longer contains the reader, but the array of paths:

[
    __DIR__ . '/../../module/xxx/src/V1/Rest',
    __DIR__ . '/../../module/xxx/src/V1/Rpc',
    __DIR__ . '/../../module/xxx/src',
]

Currently, my best guess is, that this class is instanciated with a missing first parameter. And the paths are then passed as first parameter in the readers place.

How to reproduce

doctrine/doctrine-orm-module : 5.1.0
doctrine/orm: 2.12.0
PHP : 8.0.16

Originally created by @ppaulis on GitHub (Apr 20, 2022). ### Bug Report 2.12.0 doesn't work with `doctrine/doctrine-orm-module` : 5.1.0. | Q | A |------------ | ------ | BC Break | yes? | Version | 2.12.0 #### Summary After upgrading from 2.11.1 to 2.12.0, I'm experiencing the following issue: ``` /var/www/html # vendor/bin/doctrine-module orm:generate-proxies PHP Fatal error: Uncaught Error: Call to a member function getClassAnnotations() on array in /var/www/html/vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php:82 Stack trace: #0 /var/www/html/vendor/doctrine/persistence/src/Persistence/Mapping/Driver/MappingDriverChain.php(79): Doctrine\ORM\Mapping\Driver\AnnotationDriver->loadMetadataForClass('tms\\V1\\Rest\\Sub...', Object(Doctrine\ORM\Mapping\ClassMetadata)) #1 /var/www/html/vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php(133): Doctrine\Persistence\Mapping\Driver\MappingDriverChain->loadMetadataForClass('tms\\V1\\Rest\\Sub...', Object(Doctrine\ORM\Mapping\ClassMetadata)) ``` #### Current behavior Reading Metadata no longer works through the `doctrine-orm-module`. In the following line: https://github.com/doctrine/orm/blob/d5503644314ed825fe4b276a588458a033fa3e7c/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php#L82 `$this->reader` no longer contains the reader, but the array of paths: ``` [ __DIR__ . '/../../module/xxx/src/V1/Rest', __DIR__ . '/../../module/xxx/src/V1/Rpc', __DIR__ . '/../../module/xxx/src', ] ``` Currently, my best guess is, that this class is instanciated with a missing first parameter. And the paths are then passed as first parameter in the readers place. #### How to reproduce `doctrine/doctrine-orm-module` : 5.1.0 `doctrine/orm`: 2.12.0 PHP : 8.0.16
admin added the Bug label 2026-01-22 15:42:13 +01:00
admin closed this issue 2026-01-22 15:42:13 +01:00
admin commented 2026-01-22 15:42:14 +01:00
Author
Owner
Copy Link

@ppaulis commented on GitHub (Apr 20, 2022):

Seems that this is happening here:
61dc07830d/src/Service/DriverFactory.php (L75-L76)

In my case, $class has the value Doctrine\ORM\Mapping\Driver\AnnotationDriver, but for this to work, it should probably be Doctrine\Persistence\Mapping\Driver\AnnotationDriver (which is deprecated).

I'll try to prepare a fix asap. @Ocramius

@ppaulis commented on GitHub (Apr 20, 2022): Seems that this is happening here: https://github.com/doctrine/DoctrineModule/blob/61dc07830df8eb7046dd2e791286b6e2854980e4/src/Service/DriverFactory.php#L75-L76 In my case, `$class` has the value `Doctrine\ORM\Mapping\Driver\AnnotationDriver`, but for this to work, it should probably be `Doctrine\Persistence\Mapping\Driver\AnnotationDriver` (which is deprecated). I'll try to prepare a fix asap. @Ocramius
admin commented 2026-01-22 15:42:14 +01:00
Author
Owner
Copy Link

@ppaulis commented on GitHub (Apr 20, 2022):

has already been reported here: https://github.com/doctrine/DoctrineModule/issues/774
Suggested PR : https://github.com/doctrine/DoctrineModule/pull/775

@ppaulis commented on GitHub (Apr 20, 2022): has already been reported here: https://github.com/doctrine/DoctrineModule/issues/774 Suggested PR : https://github.com/doctrine/DoctrineModule/pull/775
admin commented 2026-01-22 15:42:14 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Apr 20, 2022):

Did you also upgrade (voluntarily or not) doctrine/persistence? It just got a major release.

@greg0ire commented on GitHub (Apr 20, 2022): Did you also upgrade (voluntarily or not) `doctrine/persistence`? It just got a major release.
admin commented 2026-01-22 15:42:15 +01:00
Author
Owner
Copy Link

@ppaulis commented on GitHub (Apr 20, 2022):

@greg0ire not possible as this is a Laminas project, and doctrine/doctrine-orm-module is not (yet) compatible to doctrine/persistence: 3.0.0

@ppaulis commented on GitHub (Apr 20, 2022): @greg0ire not possible as this is a Laminas project, and `doctrine/doctrine-orm-module` is not (yet) compatible to `doctrine/persistence: 3.0.0`
admin commented 2026-01-22 15:42:15 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Apr 20, 2022):

This is caused by a BC-break I introduced in https://github.com/doctrine/orm/pull/9587, where the ORM annotation driver no longer extends the one from persistence 😬

The assumption being made here is that the abstract class we are no
longer extending is not used in type declarations and instanceof checks.

Wrong assumption, @greg0ire from the past!

@greg0ire commented on GitHub (Apr 20, 2022): This is caused by a BC-break I introduced in https://github.com/doctrine/orm/pull/9587, where the ORM annotation driver no longer extends the one from persistence :grimacing: > The assumption being made here is that the abstract class we are no longer extending is not used in type declarations and instanceof checks. Wrong assumption, @greg0ire from the past!
admin commented 2026-01-22 15:42:15 +01:00
Author
Owner
Copy Link

@Ocramius commented on GitHub (Apr 21, 2022):

Ref: https://github.com/Roave/psr-container-doctrine/issues/70

@Ocramius commented on GitHub (Apr 21, 2022): Ref: https://github.com/Roave/psr-container-doctrine/issues/70
admin commented 2026-01-22 15:42:15 +01:00
Author
Owner
Copy Link

@Ocramius commented on GitHub (Apr 21, 2022):

Suggestion on my end is to revert the offending (involuntary) BC break for now, and release 2.12.1 with that fix.

That means re-introducing some tech debt, and planning its removal for 3.0.x.

@Ocramius commented on GitHub (Apr 21, 2022): Suggestion on my end is to revert the offending (involuntary) BC break for now, and release `2.12.1` with that fix. That means re-introducing some tech debt, and planning its removal for `3.0.x`.
admin commented 2026-01-22 15:42:16 +01:00
Author
Owner
Copy Link

@ppaulis commented on GitHub (Apr 21, 2022):

@Ocramius @greg0ire As the deprecated AnnotationDriver has been removed in doctrine/persistence: 3.0, does this mean, that we need to change the dependency "doctrine/persistence": "^2.4 || ^3" to "doctrine/persistence": "^2.4?

@ppaulis commented on GitHub (Apr 21, 2022): @Ocramius @greg0ire As the deprecated AnnotationDriver has been removed in `doctrine/persistence: 3.0`, does this mean, that we need to change the dependency `"doctrine/persistence": "^2.4 || ^3"` to `"doctrine/persistence": "^2.4`?
admin commented 2026-01-22 15:42:16 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Apr 21, 2022):

In ORM? I figured it does cause an issue while trying to revert: https://github.com/doctrine/orm/pull/9670, but I think conditionally defining the class based on some condition should allow us to retain compatibility with Persistence 3.

@greg0ire commented on GitHub (Apr 21, 2022): In ORM? I figured it does cause an issue while trying to revert: https://github.com/doctrine/orm/pull/9670, but I think conditionally defining the class based on some condition should allow us to retain compatibility with Persistence 3.
admin commented 2026-01-22 15:42:16 +01:00
Author
Owner
Copy Link

@Ocramius commented on GitHub (Apr 21, 2022):

conditionally defining the class based on some condition

Depends if the consumer would observe an API change or not: remember that any API change caused by third-party packages being installed or not is still an API change -> BC break.

EDIT: anyway, I suggest:

  1. reverting
  2. releasing
  3. creating a follow-up issue with ideas around this
@Ocramius commented on GitHub (Apr 21, 2022): > conditionally defining the class based on some condition Depends if the consumer would observe an API change or not: remember that any API change caused by third-party packages being installed or not is still an API change -> BC break. EDIT: anyway, I suggest: 1. reverting 2. releasing 3. creating a follow-up issue with ideas around this
admin commented 2026-01-22 15:42:17 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Apr 21, 2022):

The API change would occur if a consumer was not pinning doctrine/persistence to 2, unlike here: https://github.com/doctrine/DoctrineModule/blob/5.2.x/composer.json#L53

I think it's fair to have this BC break in the case persistence is upgraded to 3, because if you depend on the annotation driver and it extends the API from persistence, then you depend on persistence.

I've made an attempt at the revert because it sounded like it would be fast, but it looks like we already built on top of this: https://github.com/doctrine/orm/pull/9670 , so it won't be a simple revert I'm afraid.

@greg0ire commented on GitHub (Apr 21, 2022): The API change would occur if a consumer was not pinning `doctrine/persistence` to 2, unlike here: https://github.com/doctrine/DoctrineModule/blob/5.2.x/composer.json#L53 I think it's fair to have this BC break in the case persistence is upgraded to 3, because if you depend on the annotation driver and it extends the API from persistence, then you depend on persistence. I've made an attempt at the revert because it sounded like it would be fast, but it looks like we already built on top of this: https://github.com/doctrine/orm/pull/9670 , so it won't be a simple revert I'm afraid.
admin commented 2026-01-22 15:42:17 +01:00
Author
Owner
Copy Link

@Ocramius commented on GitHub (Apr 21, 2022):

I think it's fair to have this BC break in the case persistence is upgraded to 3, because if you depend on the annotation driver and it extends the API from persistence, then you depend on persistence.

The referenced symbol (Doctrine\ORM\Mapping\Driver\AnnotationDriver) is the one in in doctrine/orm: if its structure changes due to inherited API, it is a BC break also in doctrine/orm. Changing signature of AnnotationDriver based on the third-party package is still a BC break.

A suggestion could be to:

  1. polyfill the inheritance in doctrine/orm, keep Doctrine\ORM\Mapping\Driver\AnnotationDriver parity (should be possible to check this with roave/backward-compatibility-check)
  2. deprecate Doctrine\ORM\Mapping\Driver\AnnotationDriver completely: call it "dead"
  3. create a new AnnotationDriver (completely different name, like AnnotationMappingDriver or such) and point new consumers to that
@Ocramius commented on GitHub (Apr 21, 2022): > I think it's fair to have this BC break in the case persistence is upgraded to 3, because if you depend on the annotation driver and it extends the API from persistence, then you depend on persistence. The referenced symbol (`Doctrine\ORM\Mapping\Driver\AnnotationDriver`) is the one in in `doctrine/orm`: if its structure changes due to inherited API, it is a BC break also in `doctrine/orm`. Changing signature of `AnnotationDriver` based on the third-party package is still a BC break. A suggestion could be to: 1. polyfill the inheritance in `doctrine/orm`, keep `Doctrine\ORM\Mapping\Driver\AnnotationDriver` parity (should be possible to check this with `roave/backward-compatibility-check`) 2. deprecate `Doctrine\ORM\Mapping\Driver\AnnotationDriver` completely: call it "dead" 3. create a new `AnnotationDriver` (completely different name, like `AnnotationMappingDriver` or such) and point new consumers to that
admin commented 2026-01-22 15:42:17 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Apr 21, 2022):

  1. What do you mean exactly by "polyfill the inheritance"? Do you mean defining the persistence class if it does not exist? Something like if (!class_exists(TheClass)) { require_once 'some_file.php'; } reminds me of https://dev.to/greg0ire/how-to-deprecate-a-type-in-php-48cf but maybe in our case it's unlikely AnnotationDriver is ever used in a signature.
  2. This implies finding a new name, which isn't too bad for AnnotationDriver because it's ultimately going to be removed, but if this has to be done for AttributeDriver as well, then it means we will break the naming consistency between XMLDriver, YamlDriver, PHPDriver etc.

At that point I think we would be better off merging either https://github.com/doctrine/DoctrineModule/pull/776 or https://github.com/doctrine/DoctrineModule/pull/775 and hope this is the only legitimate issue caused by this BC break.

@greg0ire commented on GitHub (Apr 21, 2022): 1. What do you mean exactly by "polyfill the inheritance"? Do you mean defining the persistence class if it does not exist? Something like `if (!class_exists(TheClass)) { require_once 'some_file.php'; }` reminds me of https://dev.to/greg0ire/how-to-deprecate-a-type-in-php-48cf but maybe in our case it's unlikely `AnnotationDriver` is ever used in a signature. 2. This implies finding a new name, which isn't too bad for `AnnotationDriver` because it's ultimately going to be removed, but if this has to be done for `AttributeDriver` as well, then it means we will break the naming consistency between `XMLDriver`, `YamlDriver`, `PHPDriver` etc. At that point I think we would be better off merging either https://github.com/doctrine/DoctrineModule/pull/776 or https://github.com/doctrine/DoctrineModule/pull/775 and hope this is the only legitimate issue caused by this BC break.
admin commented 2026-01-22 15:42:17 +01:00
Author
Owner
Copy Link

@Ocramius commented on GitHub (Apr 22, 2022):

What do you mean exactly by "polyfill the inheritance"? Do you mean defining the persistence class if it does not exist?

Correct: this allows moving to newer doctrine/persistence without breaking BC, but needs to be carefully checked.

it means we will break the naming consistency between

Consistency is still secondary to BC, heh.

At that point I think we would be better off merging either https://github.com/doctrine/DoctrineModule/pull/776 or https://github.com/doctrine/DoctrineModule/pull/775 and hope this is the only legitimate issue caused by this BC break.

The BC break is still there for anyone using doctrine/orm: remember that these modules just happen to be public projects that we are aware of, and are direct consumers of the ORM, but the source of all this is the BC break, not the consumers. All consumers will have to adapt once a new major of the ORM is out, but until then, some cruft needs to stay, for the sake of stability.

@Ocramius commented on GitHub (Apr 22, 2022): > What do you mean exactly by "polyfill the inheritance"? Do you mean defining the persistence class if it does not exist? Correct: this allows moving to newer `doctrine/persistence` without breaking BC, but needs to be carefully checked. > it means we will break the naming consistency between Consistency is still secondary to BC, heh. > At that point I think we would be better off merging either https://github.com/doctrine/DoctrineModule/pull/776 or https://github.com/doctrine/DoctrineModule/pull/775 and hope this is the only legitimate issue caused by this BC break. The BC break is still there for anyone using `doctrine/orm`: remember that these modules just happen to be public projects that we are aware of, and are direct consumers of the ORM, but the source of all this is the BC break, not the consumers. All consumers will have to adapt once a new major of the ORM is out, but until then, some cruft needs to stay, for the sake of stability.
admin commented 2026-01-22 15:42:17 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Apr 22, 2022):

The cure seems worse than the disease here. I agree that you have to think of the consumers, but they are usually pretty vocal when it comes to BC-breaks, and so far this is the only complaint we got about it. I asked other maintainers what they thought and tried hard not to steer them in any direction, and they pointed out that the code in the module will need to be fixed for the new class anyway. I think we should merge a quick fix, and ask the maintainers of the Laminas integration to rethink that piece of code entirely, and be done with it. Same goes for https://github.com/Roave/psr-container-doctrine/blob/3.6.x/src/DriverFactory.php#L52

Note that both pieces of code got worse when AttributeDriver was introduced, and that things will likely continue to get worse every time we touch this area in ORM.

@greg0ire commented on GitHub (Apr 22, 2022): The cure seems worse than the disease here. I agree that you have to think of the consumers, but they are usually pretty vocal when it comes to BC-breaks, and so far this is the only complaint we got about it. I asked other maintainers what they thought and tried hard not to steer them in any direction, and they pointed out that the code in the module will need to be fixed for the new class anyway. I think we should merge a quick fix, and ask the maintainers of the Laminas integration to rethink that piece of code entirely, and be done with it. Same goes for https://github.com/Roave/psr-container-doctrine/blob/3.6.x/src/DriverFactory.php#L52 Note that both pieces of code got worse when `AttributeDriver` was introduced, and that things will likely continue to get worse every time we touch this area in ORM.
admin commented 2026-01-22 15:42:17 +01:00
Author
Owner
Copy Link

@Ocramius commented on GitHub (Apr 22, 2022):

I'm not sure if my message is getting through: Roave/* and the doctrine/*Module are a spit in an ocean of consumers.

As I already mentioned, code will be improved where/when there is time to do so (especially when required in a new major release of doctrine/orm), but until then, BC is sacred. If you break BC, declare it with a new major, and consumers will adapt when they support the new version range.

@Ocramius commented on GitHub (Apr 22, 2022): I'm not sure if my message is getting through: `Roave/*` and the `doctrine/*Module` are a spit in an ocean of consumers. As I already mentioned, code will be improved where/when there is time to do so (especially when required in a new major release of `doctrine/orm`), but until then, BC is sacred. If you break BC, declare it with a new major, and consumers will adapt when they support the new version range.
admin commented 2026-01-22 15:42:18 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Apr 22, 2022):

a spit in an ocean of consumers

I don't doubt that, what I doubt is that there are that many type checks on AnnotationDriver. So far this has only be reported to happen in DoctrineModule and Roave/psr-container-doctrine, and it seems easier to fix both packages than implement a fix in doctrine/orm. I can certainly dedicate some time to writing a fix for psr-container-doctrine once we agree on one for DoctrineModule.

@greg0ire commented on GitHub (Apr 22, 2022): > a spit in an ocean of consumers I don't doubt that, what I doubt is that there are that many type checks on `AnnotationDriver`. So far this has only be reported to happen in `DoctrineModule` and `Roave/psr-container-doctrine`, and it seems easier to fix both packages than implement a fix in `doctrine/orm`. I can certainly dedicate some time to writing a fix for `psr-container-doctrine` once we agree on one for `DoctrineModule`.
admin commented 2026-01-22 15:42:18 +01:00
Author
Owner
Copy Link

@Ocramius commented on GitHub (Apr 22, 2022):

Be my guest in fixing symptoms.

Seriously, just fix the BC break, and all is good 😛

I'll gladly accept patches that improve the PSR integration too, but beware that this still broke all released versions anyway.

@Ocramius commented on GitHub (Apr 22, 2022): Be my guest in fixing symptoms. Seriously, just fix the BC break, and all is good 😛 I'll gladly accept patches that improve the PSR integration too, but beware that this still broke all released versions anyway.
admin commented 2026-01-22 15:42:19 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Apr 22, 2022):

Seriously, just fix the BC break, and all is good 😛

I would if it wasn't that hard, but I'm more confident in fixing the symptoms as you say 😅 Hopefully people who upgrade the ORM will also be able to upgrade both other packages 🤞

@greg0ire commented on GitHub (Apr 22, 2022): > Seriously, just fix the BC break, and all is good :stuck_out_tongue: I would if it wasn't that hard, but I'm more confident in fixing the symptoms as you say :sweat_smile: Hopefully people who upgrade the ORM will also be able to upgrade both other packages :crossed_fingers:
admin commented 2026-01-22 15:42:19 +01:00
Author
Owner
Copy Link

@derrabus commented on GitHub (Apr 22, 2022):

While I agree that changing the inheritance chain is a potential BC break, I'm not sure I 100% follow the argument of the BC break here. The code in question assumes that all driver classes that require an annotation reader are child classes of the AnnotationDriver class from doctrine/persistence and vice versa. This assumption did fail previously (doctrine/DoctrineModule#728), it fails now and is doomed to fail again.

That being said, we have to acknowledge that this flawed piece of code has existed for roughly a decade now and that we would probably need to patch multiple legacy branches of DoctrineModule because applications that use older PHP versions than 7.4 have to pin DoctrineModule to an old version while still being able to upgrade to the latest ORM.

My proposal to tackle the issue: #9671

We would still need to change DoctrineModule though, but we can do so when working on making it compatible with Persistence 3.

@derrabus commented on GitHub (Apr 22, 2022): While I agree that changing the inheritance chain is a potential BC break, I'm not sure I 100% follow the argument of the BC break here. The code in question assumes that all driver classes that require an annotation reader are child classes of the `AnnotationDriver` class from doctrine/persistence and vice versa. This assumption did fail previously (doctrine/DoctrineModule#728), it fails now and is doomed to fail again. That being said, we have to acknowledge that this flawed piece of code has existed for roughly a decade now and that we would probably need to patch multiple legacy branches of DoctrineModule because applications that use older PHP versions than 7.4 have to pin DoctrineModule to an old version while still being able to upgrade to the latest ORM. My proposal to tackle the issue: #9671 We would still need to change DoctrineModule though, but we can do so when working on making it compatible with Persistence 3.
admin commented 2026-01-22 15:42:19 +01:00
Author
Owner
Copy Link

@Ocramius commented on GitHub (Apr 22, 2022):

Awesome, thanks!

@Ocramius commented on GitHub (Apr 22, 2022): Awesome, thanks!
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
BC Break
Bug
Can't Fix
Deprecation
Documentation
DQL
Duplicate
Failing Test
Hacktoberfest
Improvement
Improvement
Incomplete
Invalid
Missing Tests
New Feature
PR Created
pull-request
Mirrored from GitHub Pull Request
 Question
Regression
Status: Waiting feedback
Waiting feedback
WIP
Won't Fix
No Label Bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
admin
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6963
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?