Skip to content

Fix some Doctrine runtime exceptions#4695

Closed
stweil wants to merge 1 commit into
vufind-org:devfrom
stweil:doctrine
Closed

Fix some Doctrine runtime exceptions#4695
stweil wants to merge 1 commit into
vufind-org:devfrom
stweil:doctrine

Conversation

@stweil
Copy link
Copy Markdown
Contributor

@stweil stweil commented Oct 7, 2025

Doctrine ORM expects an entity class, not an interface. Doctrine cannot instantiate or query interfaces—only concrete classes.

The mapping in PluginManager.php only applies to dependency injection and factory-based object creation, not to Doctrine ORM DQL queries.

@stweil
Copy link
Copy Markdown
Contributor Author

stweil commented Oct 7, 2025

I only see the exceptions in the debugger, but according to the information which I found (partially with AI support) the previous code cannot work.

Note that there are more similar code locations which also might. need a fix.

Doctrine ORM expects an entity class, not an interface.
Doctrine cannot instantiate or query interfaces—only concrete classes.

The mapping in PluginManager.php only applies to dependency injection
and factory-based object creation, not to Doctrine ORM DQL queries.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Copy link
Copy Markdown
Contributor

@EreMaijala EreMaijala left a comment

Choose a reason for hiding this comment

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

Doctrine can resolve the interfaces to the target classes using its ResolveTargetEntityListener that's initialized in module/VuFind/src/VuFind/Db/EntityManagerFactory.php. While it uses an exception internally to trigger the fallback loader, the exception won't be passed on if the interface gets resolved to an actual class.

If this was really a problem, VuFind's database functionality would not work at all. The interfaces are used everywhere.

There is a similar case of runtime exceptions happening for command-specific arguments in console utilities, and it's not a real problem either. While it can be argued that using exceptions for normal control flow is inappropriate, VuFind also does that internally e.g. to trigger an authentication request.

Copy link
Copy Markdown
Member

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

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

Please see the conversation thread on #2233 about this issue -- this is the way it is by design, and it does work correctly. I believe that we should probably close this PR without changes, but if you want to discuss the Doctrine implementation in more detail, please feel free to join a Community Call (there is one today at 13:00 GMT).

As @EreMaijala says, if we wanted to change our approach to DQL generation, we would have to make many more changes than the ones in this PR. But I do not think we want to change it, based on my comments in the thread linked above.

@stweil stweil closed this Oct 7, 2025
@stweil stweil deleted the doctrine branch October 8, 2025 13: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