Skip to content
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

Remove unused mapping services #794

Merged
merged 1 commit into from
Dec 9, 2023

Conversation

franmomu
Copy link
Contributor

@franmomu franmomu commented Dec 9, 2023

Follow-up of #793

While removing annotation mapping support in #793, I've notice that these services are not necessary.

So mapping drivers are registered in AbstractDoctrineExtension::registerMappingDrivers().

For example for attribute it executes this code which in our case uses doctrine_mongodb.odm.metadata.attribute.class to create the AttributeDriver service.

@franmomu franmomu added the Task label Dec 9, 2023
@franmomu franmomu added this to the 5.0.0 milestone Dec 9, 2023
@franmomu franmomu changed the title Removed unused mapping services Remove unused mapping services Dec 9, 2023
@franmomu franmomu force-pushed the remove_unused_mapping_services branch 2 times, most recently from f4a2f2f to 3e10c30 Compare December 9, 2023 14:01
@franmomu franmomu changed the base branch from 4.7.x to 5.0.x December 9, 2023 14:21
@franmomu franmomu force-pushed the remove_unused_mapping_services branch from 3e10c30 to 66b2869 Compare December 9, 2023 14:22
Comment on lines 33 to 34
<parameter key="doctrine_mongodb.odm.metadata.driver_chain.class">Doctrine\Persistence\Mapping\Driver\MappingDriverChain</parameter>
<parameter key="doctrine_mongodb.odm.metadata.attribute.class">Doctrine\ODM\MongoDB\Mapping\Driver\AttributeDriver</parameter>
Copy link
Member

Choose a reason for hiding this comment

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

This 2 parameters are no longer necessary.

Suggested change
<parameter key="doctrine_mongodb.odm.metadata.driver_chain.class">Doctrine\Persistence\Mapping\Driver\MappingDriverChain</parameter>
<parameter key="doctrine_mongodb.odm.metadata.attribute.class">Doctrine\ODM\MongoDB\Mapping\Driver\AttributeDriver</parameter>

You should also remove the assertions on this parameters in the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are still necessary, the thing about how it works is that (for $driverType = 'attribute' for example) this code:

$mappingDriverDef = new Definition($this->getMetadataDriverClass($driverType), [
    array_values($driverPaths),
]);

calls to our:

protected function getMetadataDriverClass(string $driverType): string
{
    return '%' . $this->getObjectManagerElementName('metadata.' . $driverType . '.class') . '%';
}

// ...

protected function getObjectManagerElementName(string $name): string
{
    return 'doctrine_mongodb.odm.' . $name;
}

so it looks for %doctrine_mongodb.odm.metadata.attribute.class%

Copy link
Member

Choose a reason for hiding this comment

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

Ok, got it.

@franmomu franmomu merged commit 9fcfc1b into doctrine:5.0.x Dec 9, 2023
13 checks passed
@franmomu franmomu deleted the remove_unused_mapping_services branch December 9, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants