Add support for file path iterables in ColocatedMappingDriver#432
Closed
rela589n wants to merge 4 commits intodoctrine:4.1.xfrom
Closed
Add support for file path iterables in ColocatedMappingDriver#432rela589n wants to merge 4 commits intodoctrine:4.1.xfrom
ColocatedMappingDriver#432rela589n wants to merge 4 commits intodoctrine:4.1.xfrom
Conversation
When checking $includedFiles for an included file, the previous `in_array()` approach, executed in a loop, is very expensive. Basically, it results in performance of O(N * M), where N - number of declared classes, M - number of included classes. The current approach is O(N), since `isset()` check has constant `O(1)` time.
e122480 to
d289c37
Compare
rela589n
added a commit
to rela589n/doctrine-mongodb-odm
that referenced
this pull request
Aug 8, 2025
In the scope of doctrine/persistence#432 (available from `doctrine/persistence` >= 4.1) there was added `ColocatedMappingDriver::$filePaths`, which allows passing an `Traversable` of file paths for the mapping driver to use. This commit integrates those changes into `AttributeDriver` (and `AnnotationDriver`). Since `doctrine/orm` maintains the support for `doctrine/persistence` of older versions, the `AttributeDriver` ensures that `$filePaths` is actually defined. Tests use `InstalledVersions` to opt into new behaviour if `doctrine/persistence` is at least version 4.1. The old behaviour can be adapted into new by using `DirectoryFilesIterator` and `FilePathNameIterator` on directory paths.
rela589n
added a commit
to rela589n/doctrine-mongodb-odm
that referenced
this pull request
Aug 8, 2025
In the scope of doctrine/persistence#432 (available from `doctrine/persistence` >= 4.1) there was added `ColocatedMappingDriver::$filePaths`, which allows passing an `Traversable` of file paths for the mapping driver to use. This commit integrates those changes into `AttributeDriver` (and `AnnotationDriver`). Since `doctrine/orm` maintains the support for `doctrine/persistence` of older versions, the `AttributeDriver` ensures that `$filePaths` is actually defined. Tests use `InstalledVersions` to opt into new behaviour if `doctrine/persistence` is at least version 4.1. The old behaviour can be adapted into new by using `DirectoryFilesIterator` and `FilePathNameIterator` on directory paths.
greg0ire
reviewed
Aug 8, 2025
9f869fb to
a241c82
Compare
rela589n
added a commit
to rela589n/doctrine-mongodb-odm
that referenced
this pull request
Aug 9, 2025
In the scope of doctrine/persistence#432 (available from `doctrine/persistence` >= 4.1) there was added `ColocatedMappingDriver::$filePaths`, which allows passing an `Traversable` of file paths for the mapping driver to use. This commit integrates those changes into `AttributeDriver` (and `AnnotationDriver`). Since `doctrine/orm` maintains the support for `doctrine/persistence` of older versions, the `AttributeDriver` ensures that `$filePaths` is actually defined. Tests use `InstalledVersions` to opt into new behaviour if `doctrine/persistence` is at least version 4.1. The old behaviour can be adapted into new by using `DirectoryFilesIterator` and `FilePathNameIterator` on directory paths.
greg0ire
previously approved these changes
Aug 9, 2025
GromNaN
reviewed
Aug 12, 2025
Member
GromNaN
left a comment
There was a problem hiding this comment.
I really like this progressive approach.
Comment on lines
+133
to
+136
| $filePathsIterator = $this->concatIterables( | ||
| $this->filePaths ?? [], | ||
| new FilePathNameIterator($dirFilesIterator), | ||
| ); |
Member
There was a problem hiding this comment.
The AppendIterator would be perfect to replace concatIterables and avoid this private method to the class that uses the trait.
Suggested change
| $filePathsIterator = $this->concatIterables( | |
| $this->filePaths ?? [], | |
| new FilePathNameIterator($dirFilesIterator), | |
| ); | |
| $filePathsIterator = new \AppendIterator(); | |
| $filePathsIterator->append(new ArrayIterator($this->filePaths ?? [])); | |
| $filePathsIterator->append(new FilePathNameIterator($dirFilesIterator)); |
Contributor
Author
There was a problem hiding this comment.
I also considered doing it this way, but the problem is that ArrayIterator doesn't accept a generic iterable (nor Traversable). Although it works well with array|ArrayObject, that's the only input type it supports. It won't work with $filePaths being some Traversable instead.
| * @template TKey | ||
| * @template T | ||
| */ | ||
| private function pathNameIterator(iterable $filesIterator): Generator |
Member
There was a problem hiding this comment.
Technically, removing a private method from a trait is a breaking change. It could be used by the class that uses it in an other package. But this code has not been released yet.
That's why I thing it's better not introducing concatIterables in this trait.
This change makes it easy for the depending libraries to use iterable of `SplFileInfo`, adapted to the format of `ColocatedMappingDriver` file paths. For example, one could provide an instance of Symfony Finder, adapted with `FilePathNameIterator` without having to reinvent it from scratch.
This change makes it easy for the client code to migrate toward the newer version. The migration would be as easy as passing `new FilePathIterator(new DirectoryFilesIterator($paths))` into the constructor instead of the array of `$paths`.
a241c82 to
a6b4bad
Compare
This commit adds support for `ColocatedMappingDriver` to accept `iterable` of file paths. Before it was only possible to accept an array of directory paths. Now, one could provide fine-grained iterator of only necessary files to the Driver. Bundles should use Symfony Finder to implement it, since it gives much flexibility to the client code to configure which files should be included and which not. Backward compatibility is achieved with the following approach: If it's an array, then it should be OK to take a look at `$paths[0]` and determine if it is a file or a directory. If it's not an array, we can assume that it's `$filePaths` iterable that's given.
a6b4bad to
a298ff5
Compare
Member
GromNaN
pushed a commit
to GromNaN/mongodb-odm
that referenced
this pull request
Aug 21, 2025
In the scope of doctrine/persistence#432 (available from `doctrine/persistence` >= 4.1) there was added `ColocatedMappingDriver::$filePaths`, which allows passing an `Traversable` of file paths for the mapping driver to use. This commit integrates those changes into `AttributeDriver` (and `AnnotationDriver`). Since `doctrine/orm` maintains the support for `doctrine/persistence` of older versions, the `AttributeDriver` ensures that `$filePaths` is actually defined. Tests use `InstalledVersions` to opt into new behaviour if `doctrine/persistence` is at least version 4.1. The old behaviour can be adapted into new by using `DirectoryFilesIterator` and `FilePathNameIterator` on directory paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR follows from #429 (comment)
ColocatedMappingDriverto acceptiterableof file paths. This enhances flexibility as clients can now provide specific iterators of required files, rather than an array of complete directories.DirectoryFilesIteratorandFilePathNameIteratorfor easier integration with iterable file paths. This benefits libraries by adaptingSplFileInfoobjects.in_array()within a loop (O(N * M)) withisset()(O(N)), providing a significant performance enhancement.