Skip to content

Feature: add support for ColocatedMappingDriver::$sourceFilePathNames #12106

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

Draft
wants to merge 1 commit into
base: 3.6.x
Choose a base branch
from

Conversation

rela589n
Copy link
Contributor

@rela589n rela589n commented Aug 5, 2025

The changes introduced add support for ColocatedMappingDriver::$sourceFilePathNames, from doctrine/persistence >= 4.1 (doctrine/persistence#429). Since doctrine/orm maintains the support for doctrine/persistence of older versions, AttributeDriver ensures that $sourceFilePathNames is actually defined. Tests use InstalledVersions to opt into new behavior if doctrine/persistence is at least version 4.1. The new behavior is adapted by using glob() on paths.

@@ -37,6 +37,7 @@
"symfony/var-exporter": "^6.3.9 || ^7.0"
},
"require-dev": {
"doctrine/persistence": "^3.3.1 || ^4.0 || 4.1.x-dev",
Copy link
Member

Choose a reason for hiding this comment

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

We should keep this MR in draft status until this can be reverted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the way, I've noticed that doctrine/DoctrineBundle has "minimum-stability": "dev", which makes possible the installation of dev versions of packages that match the constraint ("doctrine/persistence": "^3.1 || ^4" used there acquires 4.1.x-dev).

Copy link
Member

Choose a reason for hiding this comment

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

We have a CI job that tests against unstable packages (mostly Symfony). I think, you can just add Persitence there. It makes sense to test against dev versions of Persistence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean to adjust phpunit-smoke-check to include persistence version 4.1-dev?

@greg0ire greg0ire marked this pull request as draft August 5, 2025 17:37
@greg0ire greg0ire requested a review from derrabus August 5, 2025 17:37
@greg0ire
Copy link
Member

greg0ire commented Aug 5, 2025

@derrabus since you suggested this approach, I'm requesting your review specifically

@rela589n rela589n force-pushed the feat-attribute-driver-iterable-files branch 7 times, most recently from 684dbc0 to 8794b18 Compare August 7, 2025 08:01
*/
public function __construct(array $paths, bool $reportFieldsWhereDeclared = true)
public function __construct(iterable $paths, bool $reportFieldsWhereDeclared = true, bool $pathsAsFilePaths = false)
Copy link
Member

Choose a reason for hiding this comment

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

I don't fully understand why we need this new flag. Can you elaborate on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This approach was suggested by @greg0ire in doctrine/persistence#424 (comment) . This is a way of distinguishing between $paths as directory paths and as file paths. The old ColocatedMappingDriver only accepted directory paths and used RecursiveDirectoryIterator to iterate over them. The new version is complemented with iterable $sourceFilePathNames, which is an iterable of file paths. Thus, the flag is used to distinguish between the ways.

@rela589n rela589n requested a review from derrabus August 7, 2025 11:31
@rela589n rela589n force-pushed the feat-attribute-driver-iterable-files branch 2 times, most recently from 582a0b8 to 44d4b45 Compare August 7, 2025 11:46
In the scope of doctrine/persistence#429
(available from `doctrine/persistence` >= 4.1) there was added
`ColocatedMappingDriver::$sourceFilePathNames`, which allows
passing the iterable of file paths for the mapping driver to use.
This commit integrates those changes into `AttributeDriver`.
Since `doctrine/orm` maintains the support for `doctrine/persistence`
of older versions, `AttributeDriver` ensures that `$sourceFilePathNames`
is actually defined. Tests use `InstalledVersions` to opt into new
behaviour if `doctrine/persistence` is at least version 4.1.
`AttributeDriver` now accepts a new boolean to opt into the new
behaviour. This flag is used to distinguish between an array of
directory paths (the existing way) and iterable of file paths
(the new way). The old behaviour can be adapted into new by using
`glob()` search on directory paths.
@rela589n rela589n force-pushed the feat-attribute-driver-iterable-files branch from 44d4b45 to 35699bf Compare August 7, 2025 13:52
rela589n added a commit to rela589n/DoctrineBundle that referenced this pull request Aug 7, 2025
This commit adds support for a `$pathsAsFilePaths` parameter,
introduced as part of doctrine/orm#12106

It adds a minor BC break (as well as was in the case of
`$reportFieldsWhereDeclared` - `8d8579`) - classes,
extending `DoctrineOrmMappingsPass` that override
`createAttributeMappingDriver()` method will face signature mismatch.
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