-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DoctrineBridge] Add support for a driver type "attribute" #40793
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(once fabbot's patch is applied)
3a4a581
to
a5a14a4
Compare
@@ -193,6 +193,10 @@ protected function registerMappingDrivers($objectManager, ContainerBuilder $cont | |||
$args[0] = array_merge(array_values($driverPaths), $args[0]); | |||
} | |||
$mappingDriverDef->setArguments($args); | |||
} elseif ('attribute' === $driverType) { | |||
$mappingDriverDef = new Definition('%'.$this->getObjectManagerElementName('metadata.'.$driverType.'.class%'), [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this line is correct?
I suggest to not concat $driverType
.
So, should it really be:
new Definition('%'.$this->getObjectManagerElementName('metadata.attribute.class%')
Or did you mean to write:
new Definition('%'.$this->getObjectManagerElementName('metadata.attribute.class').'%'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied that from the annotation code. Yours makes more sense, but the method returns the string , so the result is the same either way. I will clean this up.
This is a duplicate of symfony/doctrine-bridge#10 (but this one looks incomplete). |
a5a14a4
to
cecaa78
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you
@@ -437,7 +441,7 @@ protected function getMetadataDriverClass(string $driverType): string | |||
{ | |||
@trigger_error(sprintf('Not declaring the "%s" method in class "%s" is deprecated since Symfony 4.4. This method will be abstract in Symfony 5.0.', __METHOD__, static::class), \E_USER_DEPRECATED); | |||
|
|||
return '%'.$this->getObjectManagerElementName('metadata.'.$driverType.'.class%'); | |||
return '%'.$this->getObjectManagerElementName('metadata.'.$driverType.'.class').'%'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. Thank you
👍 |
Thank you everyone, this makes everything easier :) |
Without this change its not possible to use attributes for mapping when they get released in ORM 2.9 over the next days. Otherwise we would need to copy three methods from the
AbstractDoctrineExtension
into the Bundle. See the DoctrineBundle PR that makes the full changes: doctrine/DoctrineBundle#1322