Skip to content

Decorator base class for object manager decorators #229

Merged
merged 1 commit into from Jan 20, 2013

6 participants

@lstrojny

As discussed on IRC, the first PR for decorator base classes. This time for ObjectManager.

@doctrinebot

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DCOM-148

@guilhermeblanco
Doctrine member

Sorry, but I don't any direct value to Doctrine project.
It may be only me, but we strongly avoid creating support that brings more maintenance, more burden to this project.
Also, it does not directly affect Doctrine, consider this as not part of Core.

@lstrojny

@guilhermeblanco While I would argue that it is crucial for Doctrine to provide well defined extension points for central classes like EntityManager, I can see your point. On a sitenote: having decorator infrastructure in Doctrine itself means that it can be supported quite nicely (add support for it via Doctrine\ORM\Configuration, extending the SF2 DoctrineBundle to support it and so on).

An alternative would be to add interfaces and setEntityManagermethods for at least the following classes:

  • Doctrine\ORM\EntityManager
  • Doctrine\ORM\Query
  • Doctrine\ORM\QueryBuilder

Than a third party bundle could add decorator support. Let me know what you think.

@schmittjoh
Doctrine member
@beberlei
Doctrine member

@guilhermeblanco We have been telling people that extending the EM should be done through decoration, yet we haven't provided this decorator base for now, making this process unnecessarily hard for users.

@stof
Doctrine member
stof commented Nov 26, 2012

@beberlei see my comment on the ORM pull request. Using composition does not work in all places as the ORM injects itself in other places (which will not inject the decorator)

@beberlei
Doctrine member

@stof yes for the EntityRepository you can create a decorator as well to fix that, however for the Event API there is no solution. But i guess that can be documented and accepted

@beberlei beberlei merged commit f44d98d into doctrine:master Jan 20, 2013

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.