Skip to content

Commit d4d7328

Browse files
jschaedljschaedl
committed
[Config] Code review changes
- added class and methods docs - fixed code style - finalzed tests
1 parent 9082ecc commit d4d7328

File tree

2 files changed

+68
-34
lines changed

2 files changed

+68
-34
lines changed

‎src/Symfony/Component/Config/Definition/Builder/NodeFinder.php‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,45 +12,44 @@
1212
namespace Symfony\Component\Config\Definition\Builder;
1313

1414
/**
15+
* This class finds nodes in a node tree.
16+
*
1517
* @author Jan Schädlich <jan.schaedlich@sensiolabs.de>
1618
*/
1719
class NodeFinder
1820
{
19-
public function find(string $nodePath, NodeDefinition $rootNode)
21+
/**
22+
* Finds a node defined by $nodePath within the given $rootNode.
23+
*
24+
* @param string $nodePath The path of the node to find. e.g "doctrine.orm.mappings"
25+
* @param NodeDefinition $rootNode
26+
*
27+
* @return NodeDefinition
28+
*/
29+
public function find($nodePath, NodeDefinition $rootNode)
2030
{
2131
if (!$rootNode instanceof ArrayNodeDefinition) {
2232
if ($nodePath !== $rootNode->getName()) {
23-
throw new \RuntimeException(
24-
sprintf(
25-
'Node with name "%s" does not exist in the given root node "%s"!',
26-
$nodePath,
27-
$rootNode->getName()
28-
)
29-
);
33+
throw new \RuntimeException(sprintf('Node with name "%s" does not exist in the given root node "%s"!', $nodePath, $rootNode->getName()));
3034
}
3135

3236
return $rootNode;
3337
}
3438

3539
$explodedNodePaths = explode('.', $nodePath);
3640

37-
if (!array_key_exists($explodedNodePaths[0], $rootNode->getChildNodeDefinitions())) {
38-
throw new \RuntimeException(
39-
sprintf(
40-
'Node with name "%s" does not exist in the given root node "%s"!',
41-
$explodedNodePaths[0],
42-
$rootNode->getName()
43-
)
44-
);
41+
if (!\array_key_exists($explodedNodePaths[0], $rootNode->getChildNodeDefinitions())) {
42+
throw new \RuntimeException(sprintf('Node with name "%s" does not exist in the given root node "%s"!', $explodedNodePaths[0], $rootNode->getName()));
4543
}
4644

4745
$node = $rootNode->getChildNodeDefinitions()[$explodedNodePaths[0]];
4846

49-
if (1 === count($explodedNodePaths)) {
47+
if (1 === \count($explodedNodePaths)) {
5048
return $node;
5149
}
5250

5351
array_shift($explodedNodePaths);
52+
5453
return $this->find(implode('.', $explodedNodePaths), $node);
5554
}
5655
}

‎src/Symfony/Component/Config/Tests/Definition/Builder/NodeFinderTest.php‎

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,16 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
1616
use Symfony\Component\Config\Definition\Builder\BooleanNodeDefinition;
17+
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
1718
use Symfony\Component\Config\Definition\Builder\NodeFinder;
1819
use Symfony\Component\Config\Definition\Builder\ScalarNodeDefinition;
19-
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
20-
use Symfony\Component\Config\Tests\Fixtures\Builder\NodeBuilder as CustomNodeBuilder;
21-
use Symfony\Component\Config\Tests\Fixtures\Builder\VariableNodeDefinition;
2220

2321
/**
2422
* @author Jan Schädlich <jan.schaedlich@sensiolabs.de>
2523
*/
2624
class NodeFinderTest extends TestCase
2725
{
28-
public function testOne()
26+
public function testItShouldFindRootNode()
2927
{
3028
$rootNode = new ScalarNodeDefinition('root');
3129

@@ -35,7 +33,34 @@ public function testOne()
3533
$this->assertSame('root', $foundNode->getName());
3634
}
3735

38-
public function testTwo()
36+
public function testItShouldThrowExceptionIfNodeDoesNotExistInScalarRootNode()
37+
{
38+
$this->expectException(\RuntimeException::class);
39+
$this->expectExceptionMessage('Node with name "child" does not exist in the given root node "root"!');
40+
41+
$rootNode = new ScalarNodeDefinition('root');
42+
43+
$nodeFinder = new NodeFinder();
44+
$nodeFinder->find('child', $rootNode);
45+
}
46+
47+
public function testItShouldThrowExceptionIfNodeDoesNotExistInArrayRootNode()
48+
{
49+
$this->expectException(\RuntimeException::class);
50+
$this->expectExceptionMessage('Node with name "child" does not exist in the given root node "root"!');
51+
52+
$rootNode = new ArrayNodeDefinition('root');
53+
$rootNode
54+
->children()
55+
->arrayNode('social_media_channels')->end()
56+
->end()
57+
;
58+
59+
$nodeFinder = new NodeFinder();
60+
$nodeFinder->find('child', $rootNode);
61+
}
62+
63+
public function testItShouldHandleComplexConfigurationProbably()
3964
{
4065
$rootNode = new ArrayNodeDefinition('root');
4166
$rootNode
@@ -58,22 +83,32 @@ public function testTwo()
5883
$nodeFinder = new NodeFinder();
5984

6085
$socialMediaChannelsNode = $nodeFinder->find('social_media_channels', $rootNode);
61-
$this->assertSame('social_media_channels', $socialMediaChannelsNode->getName());
62-
$this->assertInstanceOf(ArrayNodeDefinition::class, $socialMediaChannelsNode);
63-
6486
$socialMediaChannelsEnableNode = $nodeFinder->find('social_media_channels.enable', $rootNode);
65-
$this->assertSame('enable', $socialMediaChannelsEnableNode->getName());
66-
$this->assertInstanceOf(BooleanNodeDefinition::class, $socialMediaChannelsEnableNode);
67-
6887

88+
$twitterNode = $nodeFinder->find('social_media_channels.twitter', $rootNode);
89+
$facebookNode = $nodeFinder->find('social_media_channels.facebook', $rootNode);
90+
$instagramNode = $nodeFinder->find('social_media_channels.instagram', $rootNode);
91+
$instagramEnableNode = $nodeFinder->find('social_media_channels.instagram.enable', $rootNode);
92+
$instagramAccountsNode = $nodeFinder->find('social_media_channels.instagram.accounts', $rootNode);
6993

70-
$this->assertSame('twitter', $nodeFinder->find('social_media_channels.twitter', $rootNode)->getName());
71-
$this->assertSame('facebook', $nodeFinder->find('social_media_channels.facebook', $rootNode)->getName());
94+
$this->assertNode($socialMediaChannelsNode, 'social_media_channels', ArrayNodeDefinition::class);
95+
$this->assertNode($socialMediaChannelsEnableNode, 'enable', BooleanNodeDefinition::class);
7296

73-
$this->assertSame('instagram', $nodeFinder->find('social_media_channels.instagram', $rootNode)->getName());
74-
$this->assertSame('enable', $nodeFinder->find('social_media_channels.instagram.enable', $rootNode)->getName());
75-
$this->assertSame('accounts', $nodeFinder->find('social_media_channels.instagram.accounts', $rootNode)->getName());
97+
$this->assertNode($twitterNode, 'twitter', ArrayNodeDefinition::class);
98+
$this->assertNode($facebookNode, 'facebook', ArrayNodeDefinition::class);
99+
$this->assertNode($instagramNode, 'instagram', ArrayNodeDefinition::class);
100+
$this->assertNode($instagramEnableNode, 'enable', BooleanNodeDefinition::class);
101+
$this->assertNode($instagramAccountsNode, 'accounts', ArrayNodeDefinition::class);
76102
}
77103

78-
104+
/**
105+
* @param NodeDefinition $actualNode
106+
* @param string $expectedName
107+
* @param string $expectedType
108+
*/
109+
private function assertNode(NodeDefinition $actualNode, $expectedName, $expectedType)
110+
{
111+
$this->assertSame($expectedName, $actualNode->getName());
112+
$this->assertInstanceOf($expectedType, $actualNode);
113+
}
79114
}

0 commit comments

Comments
 (0)