Skip to content

[Config] Introduce find method in ArrayNodeDefinition to ease configuration tree manipulation #31287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 14, 2019

Conversation

jschaedl
Copy link
Contributor

@jschaedl jschaedl commented Apr 27, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #27534
License MIT
Doc PR tbd.

Description

This PR introduces a new find(string $nodePath)method in the ArrayNodeDefinition class, which helps you finding the right node to prepend configuration to ease configuration tree manipulation.

How to use it

class Configuration implements ConfigurationInterface
{
    public function getConfigTreeBuilder()
    {
        ...

        $rootNode
            ->children()
                ->arrayNode('social_media_channels')
                    ->children()
                        ->booleanNode('enable')->end()
                        ->arrayNode('twitter')->end()
                        ->arrayNode('facebook')->end()
                        ->arrayNode('instagram')->end()
                    ->end()
                ->end()
            ->end()
        ;

        $this->changeSocialMediaChannelConfiguration($rootNode->find('social_media_channels.enable'));
        $this->addTwitterConfiguration($rootNode->find('social_media_channels.twitter'));
        $this->addFacebookConfiguration($rootNode->find('social_media_channels.facebook'));
        $this->addInstagramConfiguration($rootNode->find('social_media_channels.instagram'));

        return $treeBuilder;
    }

    private function changeSocialMediaChannelConfiguration(NodeDefinition $node)
    {
        $node
            ->defaultTrue()
        ;
    }

    private function addTwitterConfiguration(NodeDefinition $node)
    {
        $node
            ->children()
                ->integerNode('client_id')->end()
                ->scalarNode('client_secret')->end()
            ->end()
        ;
    }

    private function addFacebookConfiguration(NodeDefinition $node)
    {
        $node
            ->children()
                ->integerNode('client_id')->end()
                ->scalarNode('client_secret')->end()
            ->end()
        ;
    }

    private function addInstagramConfiguration(NodeDefinition $node)
    {
        $node
            ->children()
                ->integerNode('client_id')->end()
                ->scalarNode('client_secret')->end()
            ->end()
        ;
    }
}

Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement, I like it 👌🏻
I left some minor comments

@chalasr chalasr added this to the next milestone Apr 27, 2019
@jschaedl jschaedl force-pushed the config-manipulation branch from 232d69b to d4d7328 Compare April 28, 2019 09:39
@jschaedl jschaedl force-pushed the config-manipulation branch from b0433f3 to ef7c09c Compare June 1, 2019 09:01
@jschaedl jschaedl changed the base branch from master to 4.4 June 1, 2019 09:01
@jschaedl jschaedl force-pushed the config-manipulation branch 3 times, most recently from ec0306c to 4486d96 Compare June 8, 2019 08:50
@jschaedl jschaedl force-pushed the config-manipulation branch from 4486d96 to 2522f44 Compare June 8, 2019 08:54
@jschaedl jschaedl force-pushed the config-manipulation branch 2 times, most recently from 27ab9da to f78752f Compare June 8, 2019 20:57
@jschaedl
Copy link
Contributor Author

jschaedl commented Jun 8, 2019

@ro0NL @xabbuh

I moved the find(...) method into the ArrayNodeDefinition class, removed the getter, took the pathSeparator into account and changed the path section extraction as suggested. I also adjusted the tests accordingly.

WDYT?

@jschaedl jschaedl force-pushed the config-manipulation branch from f78752f to f074903 Compare June 8, 2019 21:06
@jschaedl jschaedl force-pushed the config-manipulation branch 2 times, most recently from f964c12 to 7292bf7 Compare June 11, 2019 17:31
@jschaedl jschaedl changed the title [Config] Introduce NodeFinder to ease configuration tree manipulation [Config] Introduce find method in ArrayNodeDefinition to ease configuration tree manipulation Jun 11, 2019
@jschaedl jschaedl force-pushed the config-manipulation branch from 7292bf7 to 5c32f60 Compare June 11, 2019 17:33
@fabpot fabpot force-pushed the config-manipulation branch from cf049b5 to e3b248a Compare June 14, 2019 07:59
@fabpot
Copy link
Member

fabpot commented Jun 14, 2019

Thank you @jschaedl.

@fabpot fabpot merged commit e3b248a into symfony:4.4 Jun 14, 2019
fabpot added a commit that referenced this pull request Jun 14, 2019
…to ease configuration tree manipulation (jschaedl)

This PR was squashed before being merged into the 4.4 branch (closes #31287).

Discussion
----------

[Config] Introduce find method in ArrayNodeDefinition to ease configuration tree manipulation

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #27534   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | tbd.

### Description

This PR introduces a new `find(string $nodePath)`method in the `ArrayNodeDefinition` class, which helps you finding the right node to prepend configuration to ease configuration tree manipulation.

### How to use it
```php
class Configuration implements ConfigurationInterface
{
    public function getConfigTreeBuilder()
    {
        ...

        $rootNode
            ->children()
                ->arrayNode('social_media_channels')
                    ->children()
                        ->booleanNode('enable')->end()
                        ->arrayNode('twitter')->end()
                        ->arrayNode('facebook')->end()
                        ->arrayNode('instagram')->end()
                    ->end()
                ->end()
            ->end()
        ;

        $this->changeSocialMediaChannelConfiguration($rootNode->find('social_media_channels.enable'));
        $this->addTwitterConfiguration($rootNode->find('social_media_channels.twitter'));
        $this->addFacebookConfiguration($rootNode->find('social_media_channels.facebook'));
        $this->addInstagramConfiguration($rootNode->find('social_media_channels.instagram'));

        return $treeBuilder;
    }

    private function changeSocialMediaChannelConfiguration(NodeDefinition $node)
    {
        $node
            ->defaultTrue()
        ;
    }

    private function addTwitterConfiguration(NodeDefinition $node)
    {
        $node
            ->children()
                ->integerNode('client_id')->end()
                ->scalarNode('client_secret')->end()
            ->end()
        ;
    }

    private function addFacebookConfiguration(NodeDefinition $node)
    {
        $node
            ->children()
                ->integerNode('client_id')->end()
                ->scalarNode('client_secret')->end()
            ->end()
        ;
    }

    private function addInstagramConfiguration(NodeDefinition $node)
    {
        $node
            ->children()
                ->integerNode('client_id')->end()
                ->scalarNode('client_secret')->end()
            ->end()
        ;
    }
}

```

Commits
-------

e3b248a [Config] Introduce find method in ArrayNodeDefinition to ease configuration tree manipulation
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 2019
@jschaedl jschaedl deleted the config-manipulation branch February 23, 2020 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants