Skip to content

Commit 6b22ede

Browse files
committed
Fixes issues with environment variables not being resolved in configurations.
1 parent 0557608 commit 6b22ede

5 files changed

Lines changed: 233 additions & 116 deletions

File tree

DependencyInjection/EnvResolver.php

Lines changed: 0 additions & 80 deletions
This file was deleted.

DependencyInjection/PropelExtension.php

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,6 @@ class PropelExtension extends Extension
2929
*/
3030
public function load(array $configs, ContainerBuilder $container)
3131
{
32-
// WORKAROUND for https://github.com/symfony/symfony/issues/27683 https://github.com/symfony/symfony/issues/40906
33-
// Note that this may require the clearing of the cache after a related ENV var is changed.
34-
$configs = (new EnvResolver($container))->resolve($configs);
35-
// END WORKAROUND
36-
3732
$configuration = $this->getConfiguration($configs, $container);
3833
$config = $this->processConfiguration($configuration, $configs);
3934

@@ -57,13 +52,8 @@ public function load(array $configs, ContainerBuilder $container)
5752
throw new \InvalidArgumentException('PropelBundle expects a "phing_path" parameter that must contain the absolute path to the Phing vendor library. The "phing_path" parameter must be defined under the "propel" root node in your configuration.');
5853
}
5954

60-
if (isset($config['logging']) && $config['logging']) {
61-
$logging = $config['logging'];
62-
} else {
63-
$logging = false;
64-
}
65-
66-
$container->setParameter('propel.logging', $logging);
55+
// Set Logging Parameters
56+
$container->setParameter('propel.logging', $config['logging'] ?? false);
6757

6858
if (!empty($config['schema_path']))
6959
{
@@ -92,7 +82,12 @@ public function load(array $configs, ContainerBuilder $container)
9282
}
9383
}
9484

95-
$container->getDefinition('propel.build_properties')->setArguments(array($buildProperties));
85+
// Store the build_properties configuration at compile time, rather than injecting into service for Properties
86+
// class. This prevents issues with resolution of environment variables, which must be resolved at runtime.
87+
// The synthetic service is then set in PropelBundle::boot(). See these issues for details of problem:
88+
// https://github.com/symfony/symfony/issues/27683
89+
// https://github.com/symfony/symfony/issues/40906
90+
$container->setParameter('propel.build_properties', $buildProperties);
9691

9792
if (!empty($config['dbal'])) {
9893
$this->dbalLoad($config['dbal'], $container);
@@ -143,7 +138,12 @@ protected function dbalLoad(array $config, ContainerBuilder $container)
143138
$c['datasources']['default'] = $connectionName;
144139
}
145140

146-
$container->getDefinition('propel.configuration')->setArguments(array($c));
141+
// Store the dbal configuration at compile time, rather than creating a service for the configuration object.
142+
// This prevents issues with resolution of environment variables, which must be resolved at runtime. The
143+
// synthetic service is then set in PropelBundle::boot(). See these issues for details of problem:
144+
// https://github.com/symfony/symfony/issues/27683
145+
// https://github.com/symfony/symfony/issues/40906
146+
$container->setParameter('propel.dbal', $c);
147147
}
148148

149149
public function getConfiguration(array $config, ContainerBuilder $container)
Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
<?php
2+
3+
namespace Propel\Bundle\PropelBundle\DependencyInjection;
4+
5+
use Propel\Bundle\PropelBundle\DataCollector\PropelDataCollector;
6+
use Symfony\Component\DependencyInjection\ContainerInterface;
7+
8+
/**
9+
* Factory to create the 'propel.configuration', 'propel.data_collector', and 'propel.build_properties' services at
10+
* runtime. Services must be configured as synthetic in the bundle configuration, then the factory methods called
11+
* during the PropelBundle::boot() method.
12+
*
13+
* Class PropelConfigurationFactory
14+
*
15+
* @author AMJones <am@jonesiscoding.com>
16+
* @package Propel\Bundle\PropelBundle\DependencyInjection
17+
*/
18+
class PropelServiceFactory
19+
{
20+
const CONFIG = 'propel.configuration';
21+
const DATA_COLLECTOR = 'propel.data_collector';
22+
const BUILD_PROPERTIES = 'propel.build_properties';
23+
24+
/**
25+
* Creates a PropelDataCollector object using the 'propel.data_collector' parameter and 'propel.logger' service,
26+
* then injects the object into the container as the 'propel.build_properties' service, replacing the synthetic.
27+
*
28+
* If the container does not already have the 'propel.configuration' service, it will be created and injected into
29+
* the container as well.
30+
*
31+
* @param ContainerInterface $container
32+
*
33+
* @return PropelDataCollector|object
34+
*/
35+
public function collector(ContainerInterface $container): PropelDataCollector
36+
{
37+
$name = static::DATA_COLLECTOR;
38+
if (!$container->has($name))
39+
{
40+
// Make sure that we have a propel.configuration service
41+
$config = $this->config($container);
42+
// Get the logger Service
43+
$logger = $container->get('propel.logger');
44+
// Get the class & create object
45+
$class = $this->getClass($container, $name);
46+
$object = new $class($logger, $config);
47+
48+
$container->set($name, $object);
49+
}
50+
51+
return $container->get($name);
52+
}
53+
54+
/**
55+
* Creates a \PropelConfiguration object using the 'propel.configuration.class' and 'propel.dbal' parameters,
56+
* then injects the object into the container as the 'propel.configuration' service, replacing the synthetic.
57+
*
58+
* @param ContainerInterface $container
59+
*
60+
* @return \PropelConfiguration|object
61+
*/
62+
public function config(ContainerInterface $container): \PropelConfiguration
63+
{
64+
$name = static::CONFIG;
65+
if (!$container->has($name))
66+
{
67+
$class = $this->getClass($container, $name) ?? \PropelConfiguration::class;
68+
$dbal = $this->normalizeDbal($this->getArrayParameter($container, 'propel.dbal'));
69+
$config = new $class($dbal);
70+
71+
if ($this->isPropelLogging($container))
72+
{
73+
$config->setParameter('debugpdo.logging.methods', array(
74+
'PropelPDO::exec',
75+
'PropelPDO::query',
76+
'PropelPDO::prepare',
77+
'DebugPDOStatement::execute',
78+
), false);
79+
80+
$config->setParameter('debugpdo.logging.details', array(
81+
'time' => array('enabled' => true),
82+
'mem' => array('enabled' => true),
83+
'connection' => array('enabled' => true),
84+
));
85+
}
86+
87+
$container->set($name, $config);
88+
}
89+
90+
return $container->get($name);
91+
}
92+
93+
/**
94+
* Creates a Properties object using the 'propel.build_properties' and 'propel.build_properties.class' parameters,
95+
* then injects the object into the container as the 'propel.build_properties' service, replacing the synthetic.
96+
*
97+
* @param ContainerInterface $container
98+
*
99+
* @return Properties|object
100+
*/
101+
public function properties(ContainerInterface $container): Properties
102+
{
103+
$name = static::BUILD_PROPERTIES;
104+
if (!$container->has($name))
105+
{
106+
$class = $this->getClass($container, $name) ?? Properties::class;
107+
$props = $this->getArrayParameter($container, $name);
108+
$obj = new $class($props);
109+
110+
$container->set($name, $obj);
111+
}
112+
113+
return $container->get($name);
114+
}
115+
116+
/**
117+
* Evaluates if the 'logging' option in the propel configuration is set to TRUE.
118+
* Requires that the 'propel.logging' parameter is previously injected into the container,
119+
* typically in PropelExtension.
120+
*
121+
* @param ContainerInterface $container
122+
*
123+
* @return bool
124+
*/
125+
public function isPropelLogging(ContainerInterface $container)
126+
{
127+
return $container->hasParameter('propel.logging') && (bool)$container->getParameter('propel.logging');
128+
}
129+
130+
/**
131+
* Normalizes config settings containing the database driver. While this is also done in the configuration at
132+
* compile time, some values may be environment value placeholders, which cannot be normalized.
133+
*
134+
* @param array $dbal
135+
*
136+
* @return array
137+
*/
138+
private function normalizeDbal($dbal)
139+
{
140+
$normalizer = new DriverNormalizer();
141+
$datasources = $dbal['datasources'] ?? array();
142+
143+
foreach($datasources as $name => $datasource)
144+
{
145+
$datasources[$name] = $normalizer->normalizeDatasource($datasource);
146+
}
147+
148+
$dbal['datasources'] = $datasources;
149+
150+
return $dbal;
151+
}
152+
153+
/**
154+
* Checks for and returns a parameter as an array. Returns an empty array if the parameter
155+
* is not present in the container, or is not an array value.
156+
*
157+
* @param ContainerInterface $container
158+
* @param string $name
159+
*
160+
* @return array
161+
*/
162+
private function getArrayParameter(ContainerInterface $container, string $name): array
163+
{
164+
if ($container->hasParameter($name))
165+
{
166+
$props = $container->getParameter($name);
167+
168+
if (is_array($props))
169+
{
170+
return $props;
171+
}
172+
}
173+
174+
return array();
175+
}
176+
177+
/**
178+
* Gets the fully qualified class name for the given prefix, as long as the parameter is set in the container.
179+
*
180+
* @param ContainerInterface $container The container, which should have the <prefix>.class parameter.
181+
* @param string $prefix The string to put before .class when retrieving the parameter.
182+
*
183+
* @return string|null
184+
*/
185+
private function getClass(ContainerInterface $container, string $prefix)
186+
{
187+
$name = $prefix . '.class';
188+
if ($container->hasParameter($name))
189+
{
190+
$class = $container->getParameter($name);
191+
if (is_string($class) && class_exists($class))
192+
{
193+
return $class;
194+
}
195+
}
196+
197+
return null;
198+
}
199+
}

PropelBundle.php

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*/
1010
namespace Propel\Bundle\PropelBundle;
1111

12+
use Propel\Bundle\PropelBundle\DependencyInjection\PropelServiceFactory;
1213
use Propel\Bundle\PropelBundle\DependencyInjection\Security\UserProvider\PropelFactory;
1314
use Symfony\Component\DependencyInjection\ContainerBuilder;
1415
use Symfony\Component\HttpKernel\Bundle\Bundle;
@@ -34,26 +35,25 @@ public function boot()
3435
get_include_path());
3536
}
3637

38+
// Create the Service Factory
39+
$Factory = new PropelServiceFactory();
40+
41+
// Set the Build Properties Service
42+
$Factory->properties($this->container);
43+
44+
// Initialize Propel
3745
if (!\Propel::isInit()) {
38-
\Propel::setConfiguration($this->container->get('propel.configuration'));
46+
// Create Configuration object at runtime to prevent issues with environment variables
47+
// used in the configuration. If this is done at compile time, they aren't resolved.
48+
// https://github.com/symfony/symfony/issues/27683
49+
// https://github.com/symfony/symfony/issues/40906
50+
$config = $Factory->config($this->container);
3951

40-
if ($this->container->getParameter('propel.logging')) {
41-
$config = $this
42-
->container
43-
->get('propel.configuration')
44-
;
45-
$config->setParameter('debugpdo.logging.methods', array(
46-
'PropelPDO::exec',
47-
'PropelPDO::query',
48-
'PropelPDO::prepare',
49-
'DebugPDOStatement::execute',
50-
), false);
51-
$config->setParameter('debugpdo.logging.details', array(
52-
'time' => array('enabled' => true),
53-
'mem' => array('enabled' => true),
54-
'connection' => array('enabled' => true),
55-
));
52+
// Set the configuration created above, which has also been injected into the container as a service.
53+
\Propel::setConfiguration($config);
5654

55+
// The factory above sets the logging parameters in PropelConfiguration, if param 'propel.logging' is set.
56+
if ($Factory->isPropelLogging($this->container)) {
5757
\Propel::setLogger($this->container->get('propel.logger'));
5858
}
5959

0 commit comments

Comments
 (0)