Skip to content

[DoctrineBridge] add support for ClassLocator #61538

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.

Sign up for GitHub

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

Jump to bottom
Closed
rela589n wants to merge 2 commits into from
Closed

[DoctrineBridge] add support for ClassLocator #61538

rela589n wants to merge 2 commits into from

Conversation

@rela589n
Copy link
Contributor

@rela589n rela589n commented Aug 27, 2025

Q A
Branch? 7.4
Bug fix? no
New feature? yes
Deprecations? no
Issues #61535
License MIT

This PR introduces support for ClassLocator, added to Persistence in doctrine/persistence#433, integrated into ORM in doctrine/orm#12131, and ODM doctrine/mongodb-odm#2802.

The new AttributeDriver can now accept ClassLocator instead of $paths array:

-    public function __construct(array $paths, bool $reportFieldsWhereDeclared = true)
+    public function __construct(array|ClassLocator $paths, bool $reportFieldsWhereDeclared = true)
     {
         if (! $reportFieldsWhereDeclared) {
             throw new InvalidArgumentException(sprintf(
@@ -48,7 +49,12 @@ public function __construct(array $paths, bool $reportFieldsWhereDeclared = true
         }
 
         $this->reader = new AttributeReader();
-        $this->addPaths($paths);
+
+        if ($paths instanceof ClassLocator) {
+            $this->classLocator = $paths;
+        } else {
+            $this->addPaths($paths);
+        }
     }

The PR introduces a new ClassLocator service $driverName.'_mapping_class_locator' that will be used in the AttributeDriver.

Also it registers Finder service $driverName.'_mapping_class_finder' that is needed for the locator to operate:

$finderDefinition->addMethodCall('files');
$finderDefinition->addMethodCall('name', ['*.php']);
$finderDefinition->addMethodCall('in', [$dirs]);

This is an otherwise identical Finder-based alternative for FileClassLocator::createFromDirectories().

The changes opt in if the installed ORM/ODM libraries support it (doctrine/persistence >= 4.1, doctrine/orm >= 3.6, doctrine/mongodb-odm >= 2.12).

Replacing the array argument in $mappingDriverDef with new Reference($classLocator) could be considered a Breaking Change, since one could've modified the array of paths afterwards; however, it's unlikely that anybody used a Compiler Pass to modify it - GitHub search found no occurrences of it.

@carsonbot carsonbot added this to the 7.4 milestone Aug 27, 2025
@rela589n rela589n mentioned this pull request Aug 27, 2025
Open
@GromNaN GromNaN self-requested a review August 27, 2025 10:29
@rela589n rela589n force-pushed the 7.4-doctrine-persistence-4.1-class-locator branch 2 times, most recently from 1b5a0c6 to ad2b74b Compare August 27, 2025 12:15
GromNaN
GromNaN reviewed Aug 28, 2025
View reviewed changes
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

For the moment, I don't see what functionality it brings to the users. Is the idea to overload the $driverName.'_mapping_class_finder' service with custom constraints?

src/Symfony/Bridge/Doctrine/DependencyInjection/AbstractDoctrineExtension.php
$finderService = $this->getObjectManagerElementName($driverName.'_mapping_class_finder');

if ($container->hasDefinition($finderService)) {
$finderDefinition = $container->getDefinition($finderService);
Copy link
Member

@GromNaN GromNaN Aug 28, 2025

Choose a reason for hiding this comment

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

If the service has already be declared in the application, it should not be modified. Otherwise I fail to see the purpose of customizing this service.

Copy link
Contributor Author

@rela589n rela589n Aug 29, 2025

Choose a reason for hiding this comment

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

It's necessary to make in($dirs) call to give context of where to look for the files.

Copy link
Member

@GromNaN GromNaN Sep 2, 2025

Choose a reason for hiding this comment

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

If in([$dirs]) is forced, there's no real freedom to configure the finder. You have to mix the bundle configuration and your custom finder definition.

Instead of creating a service definition named $driverName.'_mapping_class_finder', I suggest you directly inject a definition as an argument to $driverName.'_mapping_class_locator', it is then necessary to define a full ClassLocator service, or replace its first argument if necessary.

Copy link
Contributor Author

@rela589n rela589n Sep 4, 2025

Choose a reason for hiding this comment

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

Let's wait for your changes; maybe this PR won't be necessary at all

@rela589n
Copy link
Contributor Author

rela589n commented Aug 29, 2025

Is the idea to overload the $driverName.'_mapping_class_finder' service with custom constraints?

Yes, that's the main point.

The second point is about the future deprecation of $paths array, so the ClassLocator approach would be necessary anyway.

@rela589n rela589n force-pushed the 7.4-doctrine-persistence-4.1-class-locator branch from ad2b74b to 13a8910 Compare August 29, 2025 10:01
@rela589n
[DoctrineBridge] feature: add support for ClassLocator
9c126d4 
In the scope of doctrine/persistence#433
(available from `doctrine/persistence` >= 4.1) there was added
`ColocatedMappingDriver::$classLocator`, which allows passing any
instance of `ClassLocator` for the mapping driver to use.

This commit integrates those changes into `AbstractDoctrineExtension`,
used by respective ORM, MongoDB-ODM, PHPCR-ODM bundles. The solution
registers a "mapping_class_finder" service that can be used by the
client code to customize class finding logic.

The changes come into play starting with doctrine/persistence >= 4.1,
and the actual registration happens only if `AttributeDriver` supports
`ClassLocator`.

Dependent libraries would adhere to the same interface, where
`ClassLocator` is in the first argument.

The changes were introduced for:
- ORM: doctrine/orm#12131;
- ODM: doctrine/mongodb-odm#2802;
- PHPCR ODM: doctrine/phpcr-odm#875.
@rela589n rela589n force-pushed the 7.4-doctrine-persistence-4.1-class-locator branch from 13a8910 to 9c126d4 Compare August 29, 2025 11:19
@GromNaN
Copy link
Member

GromNaN commented Oct 7, 2025

The AbstractExtension class is being deprecated and inlined into the DoctrineBundle: doctrine/DoctrineBundle#2076
That will make such change easier to implement in the bundle directly.

@xabbuh
Copy link
Member

xabbuh commented Oct 8, 2025

The AbstractDoctrineExtension class is going to be inlined into DoctrineBundle (see doctrine/DoctrineBundle#2076) and DoctrineMongoDBBundle (see doctrine/DoctrineMongoDBBundle#923). We are going to remove it from the Symfony core (see #61992 and #61993). Please submit pull requests to both bundles instead.

@xabbuh xabbuh closed this Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GromNaN GromNaN GromNaN left review comments

Assignees

No one assigned

Labels

DoctrineBridge Feature Status: Needs Review

Projects

None yet

Milestone

7.4

Development

Successfully merging this pull request may close these issues.

4 participants

@rela589n @GromNaN @xabbuh @carsonbot