Skip to content

[DoctrineBridge] Fix use "attribute" driver by default #52606

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
Merged
merged 1 commit into from
Nov 17, 2023
Merged

[DoctrineBridge] Fix use "attribute" driver by default #52606

merged 1 commit into from
Nov 17, 2023

Conversation

vtsykun
Copy link
Contributor

@vtsykun vtsykun commented Nov 15, 2023

Q A
Branch? 7.0
Bug fix? yes
New feature? -
Deprecations? -
Issues -
License MIT

When I tested 7.0-RC1 version I noticed an error of detect doctrine driver type by default

In AbstractDoctrineExtension.php line 219:
                                                                                                               
  [InvalidArgumentException]                                                                                   
  Mapping definitions for Doctrine manager "default" require at least the "type", "dir" and "prefix" options.  
                                                                                                               

Exception trace:
  at /var/www/packeton5/vendor/symfony/doctrine-bridge/DependencyInjection/AbstractDoctrineExtension.php:219
 Symfony\Bridge\Doctrine\DependencyInjection\AbstractDoctrineExtension->assertValidMappingConfiguration() at /var/www/packeton5/vendor/symfony/doctrine-bridge/DependencyInjection/AbstractDoctrineExtension.php:97
 Symfony\Bridge\Doctrine\DependencyInjection\AbstractDoctrineExtension->loadMappingInformation() at /var/www/packeton5/vendor/doctrine/doctrine-bundle/DependencyInjection/DoctrineExtension.php:837
 Doctrine\Bundle\DoctrineBundle\DependencyInjection\DoctrineExtension->loadOrmEntityManagerMappingInformation() at /var/www/packeton5/vendor/doctrine/doctrine-bundle/DependencyInjection/DoctrineExtension.php:657

my config

doctrine:
    orm:
        mappings:
            Packeton:
                is_bundle: false
                dir: '%kernel.project_dir%/src/Entity'
                prefix: 'Packeton\Entity'
                alias: Packeton

Bug was introduced here https://github.com/symfony/symfony/pull/52142/files#diff-0f981a325d8c9eea80a258c36237861d1d00b0fb62fb04f2488bf2165776c94eR94

This conditions is never executed, because $mappingConfig['type'] is false by default

$mappingConfig['type'] ??= 'attribute';

Tooltip_016

@carsonbot carsonbot added this to the 7.0 milestone Nov 15, 2023
@nicolas-grekas
Copy link
Member

Shouldn't we do this on 6.4 instead?

@vtsykun
Copy link
Contributor Author

vtsykun commented Nov 17, 2023

the PR #52142 in which this bug was introduced was for 7.0, so I think it’s enough only fixes for 7.0

@nicolas-grekas
Copy link
Member

Thank you @vtsykun.

@nicolas-grekas nicolas-grekas merged commit 2fe1c71 into symfony:7.0 Nov 17, 2023
@vtsykun vtsykun deleted the fix/sf7-doctrine branch November 17, 2023 16:10
@stof
Copy link
Member

stof commented Nov 17, 2023

I would suggest changing the default to null instead

@nicolas-grekas
Copy link
Member

@stof that was what the original patch proposed, but I reverted to false so that the diff with 6.4 remains minimal.

@stof
Copy link
Member

stof commented Nov 17, 2023

Can't we change the default also in 6.4 ?

@nicolas-grekas
Copy link
Member

We could but I don't see the point (and I see the risk :) )

@fabpot fabpot mentioned this pull request Nov 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug DoctrineBridge Status: Needs Review
Projects
None yet
Milestone
7.0
Development

Successfully merging this pull request may close these issues.
4 participants
@vtsykun @nicolas-grekas @stof @carsonbot