Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit335b11b

Browse files
committed
Not allowing autoconfigure, instanceofConditionals or defaults for ChildDefinition
The reason is that parent-child definitions are a different mechanism for "inheritance"than instanceofConditionas and defaults... creating some edge cases when trying tofigure out which settings "win". For example:Suppose a child and parent definitions are defined in different YAML files. Thechild receives public: false from its _defaults, and the parent receives public: truefrom its _defaults. Should the final child definition be public: true (so the parentoverrides the child, even though it only came from _defaults) or public: false (wherethe child wins... even though it was only set from its _defaults). Or, if the parentis explicitly set to public: true, should that override the public: false of thechild (which it got from its _defaults)? On one hand, the parent is being explicitlyset. On the other hand, the child is explicitly in a file settings _defaults publicto false. There's no correct answer.There are also problems with instanceof. The importance goes: defaults < instanceof < service definitionBut how does parent-child relationships fit into that? If a child has public: falsefrom an _instanceof, but the parent explicitly sets public: true, which wins? Shouldwe assume the parent definition wins because it's explicitly set? Or would the_instanceof win, because that's being explicitly applied to the child definition'sclass by an _instanceof that lives in the same file as that class (whereas the parentdefinition may live in a different file).Because of this,@nicolas-grekas and I (we also talked a bit to Fabien) decided thatthe complexity was growing too much. The solution is to not allow any of thesenew feature to be used by ChildDefinition objects. In other words, when you want somesort of "inheritance" for your service, you should *either* giving your service aparent *or* using defaults and instanceof. And instead of silently not applyingdefaults and instanceof to child definitions, I think it's better to scream that it'snot supported.
1 parent9d9f628 commit335b11b

File tree

36 files changed

+457
-139
lines changed

36 files changed

+457
-139
lines changed

‎src/Symfony/Component/DependencyInjection/ChildDefinition.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespaceSymfony\Component\DependencyInjection;
1313

14+
useSymfony\Component\DependencyInjection\Exception\BadMethodCallException;
1415
useSymfony\Component\DependencyInjection\Exception\InvalidArgumentException;
1516
useSymfony\Component\DependencyInjection\Exception\OutOfBoundsException;
1617

@@ -134,6 +135,26 @@ public function replaceArgument($index, $value)
134135

135136
return$this;
136137
}
138+
139+
publicfunctionsetAutoconfigured($autoconfigured)
140+
{
141+
if ($autoconfigured) {
142+
thrownewBadMethodCallException('A ChildDefinition cannot be autoconfigured.');
143+
}
144+
145+
returnparent::setAutoconfigured($autoconfigured);
146+
}
147+
148+
publicfunctionsetInstanceofConditionals(array$instanceof)
149+
{
150+
if (!empty($instanceof)) {
151+
thrownewBadMethodCallException('A ChildDefinition cannot have instanceof conditionals set on it.');
152+
}
153+
154+
returnparent::setInstanceofConditionals($instanceof);
155+
}
156+
157+
137158
}
138159

139160
class_alias(ChildDefinition::class, DefinitionDecorator::class);

‎src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,14 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults =
215215
if ($this->isLoadingInstanceof) {
216216
$definition =newChildDefinition('');
217217
}elseif ($parent =$service->getAttribute('parent')) {
218+
if (!empty($this->instanceof)) {
219+
thrownewInvalidArgumentException(sprintf('The service "%s" cannot use the "parent" option in the same file where "instanceof" configuration is defined as using both is not supported. Try moving your child definitions to a different file.',$service->getAttribute('id')));
220+
}
221+
222+
if (!empty($defaults)) {
223+
thrownewInvalidArgumentException(sprintf('The service "%s" cannot use the "parent" option in the same file where "defaults" configuration is defined as using both is not supported. Try moving your child definitions to a different file.',$service->getAttribute('id')));
224+
}
225+
218226
$definition =newChildDefinition($parent);
219227

220228
if ($value =$service->getAttribute('inherit-tags')) {
@@ -255,6 +263,10 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults =
255263
}
256264

257265
if ($value =$service->getAttribute('autoconfigure')) {
266+
if ($value &&$definitioninstanceof ChildDefinition) {
267+
thrownewInvalidArgumentException(sprintf('The service "%s" cannot have a "parent" and also have "autoconfigure". Try setting "autoconfigure=false" for the service.',$service->getAttribute('id')));
268+
}
269+
258270
$definition->setAutoconfigured(XmlUtils::phpize($value));
259271
}
260272

‎src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,14 @@ private function parseDefinition($id, $service, $file, array $defaults)
357357
if ($this->isLoadingInstanceof) {
358358
$definition =newChildDefinition('');
359359
}elseif (isset($service['parent'])) {
360+
if (!empty($this->instanceof)) {
361+
thrownewInvalidArgumentException(sprintf('The service "%s" cannot use the "parent" option in the same file where "_instanceof" configuration is defined as using both is not supported. Try moving your child definitions to a different file.',$id));
362+
}
363+
364+
if (!empty($defaults)) {
365+
thrownewInvalidArgumentException(sprintf('The service "%s" cannot use the "parent" option in the same file where "_defaults" configuration is defined as using both is not supported. Try moving your child definitions to a different file.',$id));
366+
}
367+
360368
$definition =newChildDefinition($service['parent']);
361369

362370
$inheritTag =isset($service['inherit_tags']) ?$service['inherit_tags'] : (isset($defaults['inherit_tags']) ?$defaults['inherit_tags'] :null);
@@ -518,6 +526,10 @@ private function parseDefinition($id, $service, $file, array $defaults)
518526
}
519527

520528
if (isset($service['autoconfigure'])) {
529+
if ($service['autoconfigure'] &&$definitioninstanceof ChildDefinition) {
530+
thrownewInvalidArgumentException(sprintf('The service "%s" cannot have a "parent" and also have "autoconfigure". Try setting "autoconfigure: false" for the service.',$id));
531+
}
532+
521533
$definition->setAutoconfigured($service['autoconfigure']);
522534
}
523535

‎src/Symfony/Component/DependencyInjection/Tests/ChildDefinitionTest.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
usePHPUnit\Framework\TestCase;
1515
useSymfony\Component\DependencyInjection\ChildDefinition;
1616
useSymfony\Component\DependencyInjection\DefinitionDecorator;
17+
useSymfony\Component\DependencyInjection\Exception\BadMethodCallException;
18+
useSymfony\Component\DependencyInjection\Exception\InvalidArgumentException;
1719

1820
class ChildDefinitionTestextends TestCase
1921
{
@@ -132,4 +134,22 @@ public function testDefinitionDecoratorAliasExistsForBackwardsCompatibility()
132134
{
133135
$this->assertInstanceOf(ChildDefinition::class,newDefinitionDecorator('foo'));
134136
}
137+
138+
/**
139+
* @expectedException BadMethodCallException
140+
*/
141+
publicfunctiontestCannotCallSetAutoconfigured()
142+
{
143+
$def =newChildDefinition('foo');
144+
$def->setAutoconfigured(true);
145+
}
146+
147+
/**
148+
* @expectedException BadMethodCallException
149+
*/
150+
publicfunctiontestCannotCallSetInstanceofConditionals()
151+
{
152+
$def =newChildDefinition('foo');
153+
$def->setInstanceofConditionals(array('Foo' =>newChildDefinition('')));
154+
}
135155
}

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php‎

Lines changed: 90 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -117,64 +117,108 @@ public function testProcessInlinesWhenThereAreMultipleReferencesButFromTheSameDe
117117
$this->assertFalse($container->hasDefinition('c'),'Service C was not inlined.');
118118
}
119119

120-
publicfunctiontestInstanceofDefaultsAndParentDefinitionResolution()
120+
publicfunctiontestInstanceofDefaultsDefinitionResolution()
121121
{
122122
$container =newContainerBuilder();
123123
$container->setResourceTracking(false);
124124

125125
// loading YAML with an expressive test-case in that file
126126
$loader =newYamlFileLoader($container,newFileLocator(__DIR__.'/../Fixtures/yaml'));
127-
$loader->load('services_defaults_instanceof_parent.yml');
127+
$loader->load('integration_services_defaults_instanceof.yml');
128128
$container->compile();
129129

130-
// instanceof overrides defaults
131-
$simpleService =$container->getDefinition('service_simple');
132-
$this->assertFalse($simpleService->isAutowired());
133-
$this->assertFalse($simpleService->isAutoconfigured());
134-
$this->assertFalse($simpleService->isShared());
135-
136-
// all tags are kept
137-
$this->assertEquals(
138-
array(
139-
'foo_tag' =>array(array('tag_option' =>'from_service'),array('tag_option' =>'from_instanceof')),
140-
'bar_tag' =>array(array()),
141-
),
142-
$simpleService->getTags()
130+
$serviceFoo =$container->getDefinition('service_foo');
131+
$serviceFooExpected =$container->getDefinition('service_foo_expected');
132+
133+
// reset changes, we don't care if these differ
134+
$serviceFoo->setChanges(array());
135+
$serviceFooExpected->setChanges(array());
136+
137+
$this->assertEquals($serviceFooExpected,$serviceFoo);
138+
}
139+
140+
/**
141+
* @dataProvider getYamlCompileTests
142+
*/
143+
publicfunctiontestYamlContainerCompiles($directory,$actualServiceId,$expectedServiceId,ContainerBuilder$mainContainer =null)
144+
{
145+
// allow a container to be passed in, which might have autoconfigure settings
146+
$container =$mainContainer ?$mainContainer :newContainerBuilder();
147+
$container->setResourceTracking(false);
148+
$loader =newYamlFileLoader($container,newFileLocator(__DIR__.'/../Fixtures/yaml/integration/'.$directory));
149+
$loader->load('main.yml');
150+
$container->compile();
151+
$actualService =$container->getDefinition($actualServiceId);
152+
153+
// create a fresh ContainerBuilder, to avoid autoconfigure stuff
154+
$container =newContainerBuilder();
155+
$container->setResourceTracking(false);
156+
$loader =newYamlFileLoader($container,newFileLocator(__DIR__.'/../Fixtures/yaml/integration/'.$directory));
157+
$loader->load('expected.yml');
158+
$container->compile();
159+
$expectedService =$container->getDefinition($expectedServiceId);
160+
161+
// reset changes, we don't care if these differ
162+
$actualService->setChanges(array());
163+
$expectedService->setChanges(array());
164+
165+
$this->assertEquals($expectedService,$actualService);
166+
}
167+
168+
publicfunctiongetYamlCompileTests()
169+
{
170+
$container =newContainerBuilder();
171+
$container->registerForAutoconfiguration(IntegrationTestStub::class)
172+
->addMethodCall('enableSummer',array(true));
173+
yieldarray(
174+
'autoconfigure_child_not_applied',
175+
'child_service',
176+
'child_service_expected',
177+
$container
178+
);
179+
180+
$container =newContainerBuilder();
181+
$container->registerForAutoconfiguration(IntegrationTestStub::class)
182+
->addMethodCall('enableSummer',array(true));
183+
yieldarray(
184+
'autoconfigure_parent_child',
185+
'child_service',
186+
'child_service_expected',
187+
$container
188+
);
189+
190+
$container =newContainerBuilder();
191+
$container->registerForAutoconfiguration(IntegrationTestStub::class)
192+
->addTag('from_autoconfigure');
193+
yieldarray(
194+
'autoconfigure_parent_child_tags',
195+
'child_service',
196+
'child_service_expected',
197+
$container
198+
);
199+
200+
yieldarray(
201+
'child_parent',
202+
'child_service',
203+
'child_service_expected'
204+
);
205+
206+
yieldarray(
207+
'defaults_instanceof_importance',
208+
'main_service',
209+
'main_service_expected'
143210
);
144211

145-
// calls are all kept, but service-level calls are last
146-
$this->assertEquals(
147-
array(
148-
// from instanceof
149-
array('setSunshine',array('bright')),
150-
// from service
151-
array('enableSummer',array(true)),
152-
array('setSunshine',array('warm')),
153-
),
154-
$simpleService->getMethodCalls()
212+
yieldarray(
213+
'defaults_parent_child',
214+
'child_service',
215+
'child_service_expected'
155216
);
156217

157-
// service override instanceof
158-
$overrideService =$container->getDefinition('service_override_instanceof');
159-
$this->assertTrue($overrideService->isAutowired());
160-
$this->assertTrue($overrideService->isAutoconfigured());
161-
162-
// children definitions get no instanceof
163-
$childDef =$container->getDefinition('child_service');
164-
$this->assertEmpty($childDef->getTags());
165-
166-
$childDef2 =$container->getDefinition('child_service_with_parent_instanceof');
167-
// taken from instanceof applied to parent
168-
$this->assertFalse($childDef2->isAutowired());
169-
// override the instanceof
170-
$this->assertTrue($childDef2->isShared());
171-
// tags inherit like normal
172-
$this->assertEquals(
173-
array(
174-
'foo_tag' =>array(array('tag_option' =>'from_child_def'),array('tag_option' =>'from_parent_def'),array('tag_option' =>'from_instanceof')),
175-
'bar_tag' =>array(array()),
176-
),
177-
$childDef2->getTags()
218+
yieldarray(
219+
'instanceof_parent_child',
220+
'child_service',
221+
'child_service_expected'
178222
);
179223
}
180224
}

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,6 @@ public function testProcessInheritance()
5656
));
5757

5858
$def = (newChildDefinition('parent'))->setClass(self::class);
59-
$def->setInstanceofConditionals(array(
60-
parent::class => (newChildDefinition(''))->addMethodCall('foo',array('baz')),
61-
));
6259
$container->setDefinition('child',$def);
6360

6461
(newResolveInstanceofConditionalsPass())->process($container);

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services29.xml‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,5 @@
88
<serviceid="with_defaults"class="Foo" />
99
<serviceid="no_defaults"class="Foo"public="true"autowire="false"inherit-tags="false">
1010
</service>
11-
<serviceid="no_defaults_child"class="Foo"parent="no_defaults">
12-
<tagname="bar" />
13-
</service>
14-
<serviceid="with_defaults_child"class="Foo"parent="with_defaults"public="true"inherit-tags="true">
15-
<tagname="baz" />
16-
</service>
1711
</services>
1812
</container>
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<containerxmlns="http://symfony.com/schema/dic/services"xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
3+
<services>
4+
<serviceid="parent_service"class="stdClass" />
5+
<serviceid="child_service"class="stdClass"parent="parent_service"autoconfigure="true" />
6+
</services>
7+
</container>
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<containerxmlns="http://symfony.com/schema/dic/services"xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
3+
<services>
4+
<defaultsautowire="true" />
5+
6+
<serviceid="parent_service"class="stdClass" />
7+
<serviceid="child_service"parent="parent_service" />
8+
</services>
9+
</container>
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<containerxmlns="http://symfony.com/schema/dic/services"xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
3+
<services>
4+
<instanceofid="FooInterface"autowire="true" />
5+
6+
<serviceid="parent_service"class="stdClass" />
7+
<serviceid="child_service"class="stdClass"parent="parent_service" />
8+
</services>
9+
</container>

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp