Skip to content
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

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

Merged
merged 1 commit into from Jun 14, 2019

Conversation

@jschaedl
Copy link
Contributor

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()
        ;
    }
}
@OskarStark
Copy link
Contributor

left a comment

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 jschaedl:config-manipulation branch from 232d69b to d4d7328 Apr 28, 2019

@jschaedl jschaedl force-pushed the jschaedl:config-manipulation branch from b0433f3 to ef7c09c Jun 1, 2019

@jschaedl jschaedl changed the base branch from master to 4.4 Jun 1, 2019

@jschaedl jschaedl force-pushed the jschaedl:config-manipulation branch 3 times, most recently from ec0306c to 4486d96 Jun 1, 2019

@jschaedl jschaedl requested review from dunglas, lyrixx, sroze and xabbuh as code owners Jun 8, 2019

@jschaedl jschaedl force-pushed the jschaedl:config-manipulation branch from 4486d96 to 2522f44 Jun 8, 2019

@jschaedl jschaedl force-pushed the jschaedl:config-manipulation branch 2 times, most recently from 27ab9da to f78752f Jun 8, 2019

@jschaedl

This comment has been minimized.

Copy link
Contributor Author

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 jschaedl:config-manipulation branch from f78752f to f074903 Jun 8, 2019

@jschaedl jschaedl force-pushed the jschaedl:config-manipulation branch 2 times, most recently from f964c12 to 7292bf7 Jun 9, 2019

@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 jschaedl:config-manipulation branch from 7292bf7 to 5c32f60 Jun 11, 2019

@xabbuh

xabbuh approved these changes Jun 11, 2019

@ro0NL

ro0NL approved these changes Jun 11, 2019

@fabpot

fabpot approved these changes Jun 14, 2019

@fabpot fabpot force-pushed the jschaedl:config-manipulation branch from cf049b5 to e3b248a Jun 14, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

Thank you @jschaedl.

@fabpot fabpot merged commit e3b248a into symfony:4.4 Jun 14, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Jun 14, 2019

feature #31287 [Config] Introduce find method in ArrayNodeDefinition …
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.