Use manager registry instead of document manager
fazy opened this issue · 2 comments
I thought I should log this as an issue before going ahead with any changes:
In RoutingBundle PRs 97 and 99, classes that use a document manager now require ManagerRegistry $managerRegistry as a constructor argument and a call to a new method setManagerName($managerName).
I propose to make a similar change to BlockBundle in order to be more consistent with RoutingBundle, updating the functionality that was added in PR 54.
Affected class: PHPCRBlockLoader
Affected service: cmf.block.service
New config item: manager_registry
Possible concerns:
- The class is called "PHPCRBlockLoader", so does it make sense to make it more generic/able to load from different object managers? If so, should it be renamed?
- RoutingBundle has the config item
manager_name, while BlockBundle hasdocument_manager_name, albeit at different levels in the config tree. If these are equivalent, should one be renamed?
Just want to make sure it's worth doing, and that we don't cause any problems. :)
+1
the difference is that the routing loaders could be used as base class
for other doctrine variants, while PHPCRBlockLoader obviously is only
meant for the phpcr-odm. nontheless the need to choose the manager
(meaning the session) still makes sense here too. when you are at it,
lets rename document_manager_name to manager_name, its conceptually the
same thing.
cheers,david