From 604193396f332d9a96024b58c119cf0bd18bf7b7 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 12 Jul 2013 17:56:14 -0400 Subject: [PATCH 1/2] Remove "prototype" from abstract definition IDs; note possible conflicts Due to the naming of transformer, listener, and finder services, it's possible for index/type services to clobber the ID of another concrete or abstract service. This cannot be helped without breaking BC, but we should note it within the extension class. --- DependencyInjection/FOSElasticaExtension.php | 29 ++++++++++++-------- Resources/config/config.xml | 8 +++--- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/DependencyInjection/FOSElasticaExtension.php b/DependencyInjection/FOSElasticaExtension.php index 0cb5c27..4b90e91 100644 --- a/DependencyInjection/FOSElasticaExtension.php +++ b/DependencyInjection/FOSElasticaExtension.php @@ -144,14 +144,15 @@ class FOSElasticaExtension extends Extension */ protected function loadIndexFinder(ContainerBuilder $container, $name, $indexId) { - $abstractTransformerId = 'fos_elastica.elastica_to_model_transformer.collection.prototype'; + /* Note: transformer services may conflict with "collection.index", if + * an index and type names were "collection" and an index, respectively. + */ $transformerId = sprintf('fos_elastica.elastica_to_model_transformer.collection.%s', $name); - $transformerDef = new DefinitionDecorator($abstractTransformerId); + $transformerDef = new DefinitionDecorator('fos_elastica.elastica_to_model_transformer.collection'); $container->setDefinition($transformerId, $transformerDef); - $abstractFinderId = 'fos_elastica.finder.prototype'; $finderId = sprintf('fos_elastica.finder.%s', $name); - $finderDef = new DefinitionDecorator($abstractFinderId); + $finderDef = new DefinitionDecorator('fos_elastica.finder'); $finderDef->replaceArgument(0, new Reference($indexId)); $finderDef->replaceArgument(1, new Reference($transformerId)); @@ -262,6 +263,9 @@ class FOSElasticaExtension extends Extension if (isset($typeConfig['elastica_to_model_transformer']['service'])) { return $typeConfig['elastica_to_model_transformer']['service']; } + /* Note: transformer services may conflict with "prototype.driver", if + * the index and type names were "prototype" and a driver, respectively. + */ $abstractId = sprintf('fos_elastica.elastica_to_model_transformer.prototype.%s', $typeConfig['driver']); $serviceId = sprintf('fos_elastica.elastica_to_model_transformer.%s.%s', $indexName, $typeName); $serviceDef = new DefinitionDecorator($abstractId); @@ -285,9 +289,9 @@ class FOSElasticaExtension extends Extension if (isset($typeConfig['model_to_elastica_transformer']['service'])) { return $typeConfig['model_to_elastica_transformer']['service']; } - $abstractId = sprintf('fos_elastica.model_to_elastica_transformer.prototype.auto'); + $serviceId = sprintf('fos_elastica.model_to_elastica_transformer.%s.%s', $indexName, $typeName); - $serviceDef = new DefinitionDecorator($abstractId); + $serviceDef = new DefinitionDecorator('fos_elastica.model_to_elastica_transformer'); $serviceDef->replaceArgument(0, array( 'identifier' => $typeConfig['identifier'] )); @@ -298,9 +302,8 @@ class FOSElasticaExtension extends Extension protected function loadObjectPersister(array $typeConfig, Definition $typeDef, ContainerBuilder $container, $indexName, $typeName, $transformerId) { - $abstractId = sprintf('fos_elastica.object_persister.prototype'); $serviceId = sprintf('fos_elastica.object_persister.%s.%s', $indexName, $typeName); - $serviceDef = new DefinitionDecorator($abstractId); + $serviceDef = new DefinitionDecorator('fos_elastica.object_persister'); $serviceDef->replaceArgument(0, $typeDef); $serviceDef->replaceArgument(1, new Reference($transformerId)); $serviceDef->replaceArgument(2, $typeConfig['model']); @@ -315,7 +318,9 @@ class FOSElasticaExtension extends Extension if (isset($typeConfig['provider']['service'])) { return $typeConfig['provider']['service']; } - + /* Note: provider services may conflict with "prototype.driver", if the + * index and type names were "prototype" and a driver, respectively. + */ $providerId = sprintf('fos_elastica.provider.%s.%s', $indexName, $typeName); $providerDef = new DefinitionDecorator('fos_elastica.provider.prototype.' . $typeConfig['driver']); $providerDef->addTag('fos_elastica.provider', array('index' => $indexName, 'type' => $typeName)); @@ -333,6 +338,9 @@ class FOSElasticaExtension extends Extension if (isset($typeConfig['listener']['service'])) { return $typeConfig['listener']['service']; } + /* Note: listener services may conflict with "prototype.driver", if the + * index and type names were "prototype" and a driver, respectively. + */ $abstractListenerId = sprintf('fos_elastica.listener.prototype.%s', $typeConfig['driver']); $listenerId = sprintf('fos_elastica.listener.%s.%s', $indexName, $typeName); $listenerDef = new DefinitionDecorator($abstractListenerId); @@ -384,9 +392,8 @@ class FOSElasticaExtension extends Extension if (isset($typeConfig['finder']['service'])) { $finderId = $typeConfig['finder']['service']; } else { - $abstractFinderId = 'fos_elastica.finder.prototype'; $finderId = sprintf('fos_elastica.finder.%s.%s', $indexName, $typeName); - $finderDef = new DefinitionDecorator($abstractFinderId); + $finderDef = new DefinitionDecorator('fos_elastica.finder'); $finderDef->replaceArgument(0, $typeDef); $finderDef->replaceArgument(1, new Reference($elasticaToModelId)); $container->setDefinition($finderId, $finderDef); diff --git a/Resources/config/config.xml b/Resources/config/config.xml index a0819fb..1540269 100644 --- a/Resources/config/config.xml +++ b/Resources/config/config.xml @@ -43,26 +43,26 @@ - + - + - + - + From 74d993b64206e0e230d576928616c43fb7a3a94d Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 12 Jul 2013 17:58:13 -0400 Subject: [PATCH 2/2] Do not clobber existing client definitions (closes #336 and #324) While we could have used an abstract definition, its ID would likely conflict with the alias we set for the default client. Remove the abstract definition altogether and simply construct new definitions for each client. This resolves the previous issue where multiple clients would overwrite the constructor arguments of the previous definition. --- DependencyInjection/FOSElasticaExtension.php | 5 ++--- Resources/config/config.xml | 7 ------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/DependencyInjection/FOSElasticaExtension.php b/DependencyInjection/FOSElasticaExtension.php index 4b90e91..f264d7c 100644 --- a/DependencyInjection/FOSElasticaExtension.php +++ b/DependencyInjection/FOSElasticaExtension.php @@ -70,10 +70,9 @@ class FOSElasticaExtension extends Extension { $clientIds = array(); foreach ($clients as $name => $clientConfig) { - $clientDef = $container->getDefinition('fos_elastica.client'); - $clientDef->replaceArgument(0, $clientConfig); - $clientId = sprintf('fos_elastica.client.%s', $name); + $clientDef = new Definition('%fos_elastica.client.class%', array($clientConfig)); + $clientDef->addMethodCall('setLogger', array(new Reference('fos_elastica.logger'))); $container->setDefinition($clientId, $clientDef); diff --git a/Resources/config/config.xml b/Resources/config/config.xml index 1540269..dfb417d 100644 --- a/Resources/config/config.xml +++ b/Resources/config/config.xml @@ -27,13 +27,6 @@ - - - - - - -