Skip to content

[OptionsResolver] Support nested options #4833

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

Closed
webmozart opened this issue Jul 10, 2012 · 19 comments
Closed

[OptionsResolver] Support nested options #4833

webmozart opened this issue Jul 10, 2012 · 19 comments

Comments

@webmozart
Copy link
Contributor

It is currently not possible to configure nested options. For example:

$resolver->setDefaults(array(
    'db' => array(
        'dsn' => null,
        'user' => 'root,
        'password' => '',
        'port': 3306,
    ),
));

It is not possible to resolve this configuration in the following way:

$resolver->resolve(array(
    'db' => array(
        'dsn' => ...
    ),
));

because now the other default values under "db" will be dropped. Also, it is not possible to specify allowed values or types for the nested options.

@blaugueux
Copy link
Contributor

@bschussek Any news about this?

@webmozart
Copy link
Contributor Author

@blaugueux Not yet. Maybe this can be tackled in 2.2.

@dlsniper
Copy link
Contributor

@bschussek if you can guide me thru this I could help out and implement the changes, seeing this is a long standing issue.

Thanks!

@mtotheikle
Copy link

@bschussek ping on this issue.

@dlsniper Have you done any work on this?

@dlsniper
Copy link
Contributor

dlsniper commented Apr 5, 2013

@mtotheikle sorry, I didn't got any answer from @bschussek and currently I'm pretty booked as well for at least one more month. If you can do it, go ahead :)

Regards.

@mtotheikle
Copy link

If this hasn't been started I may be able to look at it over the next week or so, but would need some ideas for how it should be implemented.

Any ideas @dlsniper?

@shouze
Copy link

shouze commented Aug 8, 2013

@bschussek does OptionResolver really need this feature? Should be great but can remove the simplicity of the component too.

After a year or so, what is your opinion about this PR?

@stof
Copy link
Member

stof commented Aug 8, 2013

I think it would be useful. There is a few array options in the form component which would benefit from it attr or the options option of the CollectionType for instance

@webmozart
Copy link
Contributor Author

I agree with both of you. It's useful, but it's hard to implement while still keeping everything simple.

@shouze
Copy link

shouze commented Aug 17, 2013

We can give a try, we'll see if things can keep simple. Otherwise the produced code would be re arranged in another component with OptionsResolver as a dependency for example.

@acrobat
Copy link
Contributor

acrobat commented Nov 29, 2013

any news on this feature? I'm have a similar problem with the attr option in the symfony2 forms, default attr options get overwriten by attributes set in the formConfig

@sstok
Copy link
Contributor

sstok commented Nov 30, 2013

One thing I can think of at this moment is to add setNestedOption(array $options) where the name is the option and the value is an OptionsResolver instance.

$optionsResolver = new OptionsResolver();
$nestedOptionsResolver = new OptionsResolver();

$optionsResolver->setNestedOption(array('option_nested' => $nestedOptionsResolver));

When an value with key 'option_nested' is passed to $optionsResolver->resolve() it will delegate the values to the nested options resolver which will then resolve them.

One minor detail is parent dependency, as some options are lazy evaluated with a Closure they/may should have access to the parent options resolver to check some values for the final value.

And compatibility, we cant expend the Interface as this a BC, so we should either introduce an additional interface or come-up with a with way to reuse the current interface.

@chadwickmeyer
Copy link

I'm new to symfony and the open source community, so I'm not certain about protocol here. And I don't fully understand how this resolver works. But I would very much like to see setDefaults work on nested arrays, so that I can pass in options that are grouped in a sane way (rather than all on the same single level), e.g.

$resolver->setDefaults(array(
'options' => array(
    'enableField'       => array(
        'title'         => FALSE,
        'versionNotes'  => FALSE,
        'timeCustom'    => FALSE
),
    'enableAction'      => array(
        'publish'       => FALSE
    ),
    'enableEntity'      => array(
        'viewSettings'  => FALSE,
        'content'       => FALSE
    )
));

Currently OptionsResolver#resolve() does a foreach() through the options and does a set() which calls Options#overload. So why doesn't overload check if an option is an array and do an array_replace_recursive?

@stof
Copy link
Member

stof commented Nov 21, 2014

@chadwickmeyer is not not only about defaults. It is also about all the logic resolving options. And we cannot simply make it recurse when it finds an array. This would break existing cases where options are accepting arrays as values (for instance choices or valdiation_groups) rather than as a set of nested options, and so must not be merged with the previous default.
We need to find a way to let the option resolver know that it is a nested option.

@chadwickmeyer
Copy link

Thanks for responding. If I knew the whole system better, I would love to offer my help. But I am not qualified yet to understand the big picture (which is required to implement this feature). For the time being I found another way to do the merging in my application, and then I send all the defaults and the custom values to the form builder, since they have already been merged in my application .

HeahDude added a commit to HeahDude/symfony that referenced this issue Mar 12, 2016
closes symfony#4833.

Allow to resolve nested options.
HeahDude added a commit to HeahDude/symfony that referenced this issue Mar 12, 2016
closes symfony#4833.

Allow to resolve nested options.
HeahDude added a commit to HeahDude/symfony that referenced this issue Mar 12, 2016
closes symfony#4833.

Allow to resolve nested options.
HeahDude added a commit to HeahDude/symfony that referenced this issue Mar 13, 2016
closes symfony#4833.

Allow to resolve nested options.
HeahDude added a commit to HeahDude/symfony that referenced this issue Mar 13, 2016
closes symfony#4833.

Allow to resolve nested options.
HeahDude added a commit to HeahDude/symfony that referenced this issue Mar 14, 2016
closes symfony#4833.

Allow to resolve nested options.

  * add `OptionsResolver::nested`
  * add `OptionsResolver::setNested()`
  * add `OptionsResolver::isNested()`
  * add `OptionsResolver::getNested()`
@Yoshix
Copy link

Yoshix commented Jun 3, 2016

This might be overly simplistic, but wouldn't it suffice to automatically resolve an OptionsResolver that is used as a default value? E.g.:

$nested3rdLevel = new OptionsResolver();
$nested3rdLevel->setDefault('3rd', 1);

$nested2ndLevel = new OptionsResolver();
$nested2ndLevel->setDefault('2nd', $nested3rdLevel);

$nested1stLevel = new OptionsResolver();
$nested1stLevel->setDefault('1st', $nested2ndLevel);

$options = $nested1stLevel->resolve([]); //  ["1st" => ["2nd" => ["3rd" => 1]]]

The only bc-break this would introduce is that an OptionsResolver could not be used as a default value (though this could be fixed by using some internal flag). Which seems to me, is a rare use-case anyway.

I actually have a (seemingly) working implementation for the above (minus access to the parent resolver), though I wanted to first ask if this approach would be acceptable at all?

@webmozart
Copy link
Contributor Author

I will close this issue. It's been open for a while and I don't think that this is something that the OptionsResolver really should support. I also don't see a way to do this without making the implementation overly complex.

@Gummibeer
Copy link

For all interested: I've found this package sauls/options-resolver which does support deep options resolving by dot.notation in configuration.

@yceruto
Copy link
Member

yceruto commented Sep 12, 2018

See #27291 for full implementation of the initial request. Could this request be reopened, please?

fabpot added a commit that referenced this issue Oct 10, 2018
…finition (yceruto)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[OptionsResolver] Added support for nesting options definition

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #4833
| License       | MIT
| Doc PR        | symfony/symfony-docs#9995

I'd like to propose an alternative to #27251 and #18134 with a different approach.

It would allow you to create a nested options system with required options, validation (type, value),
normalization and more.

<details>
 <summary><strong>Short documentation</strong></summary>

**To define a nested option, you can pass a closure as the default value of the option with an `OptionsResolver` argument:**
```php
$resolver
    ->defaults([
        'connection' => 'default',
        'database' => function (OptionsResolver $resolver) {
            $resolver
                ->setRequired(['dbname', 'host', ...])
                ->setDefaults([
                    'driver' => 'pdo_sqlite',
                    'port' => function (Options $options) {
                        return 'pdo_mysql' === $options['driver'] ? 3306 : null,
                    },
                    'logging' => true,
                ])
                ->setAllowedValues('driver', ['pdo_sqlite', 'pdo_mysql'])
                ->setAllowedTypes('port', 'int')
                ->setAllowedTypes('logging', 'bool')
                // ...
        },
    ]);

$resolver->resolve(array(
    'database' => array(
        'dbname' => 'demo',
        'host' => 'localhost',
        'driver' => 'pdo_mysql',
    ),
));

// returns: array(
//    'connection' => 'default',
//    'database' => array(
//        'dbname' => 'demo',
//        'host' => 'localhost',
//        'driver' => 'pdo_mysql',
//        'port' => 3306,
//        'logging' => true,
//    ),
//)
```
Based on this instance, you can define the options under ``database`` and its desired default
value.

**If the default value of a child option depend on another option defined in parent level,
adds a second ``Options`` argument to the closure:**
```php
$resolver
    ->defaults([
        'profiling' => false,
        'database' => function (OptionsResolver $resolver, Options $parent) {
            $resolver
                ->setDefault('logging', $parent['profiling'])
                ->setAllowedTypes('logging', 'bool');
        },
    ])
;
```
**Access to nested options from lazy or normalize functions in parent level:**
```php
$resolver
    ->defaults([
        'version' => function (Options $options) {
            return $options['database']['server_version'];
        },
        'database' => function (OptionsResolver $resolver) {
            $resolver
                ->setDefault('server_version', 3.15)
                ->setAllowedTypes('server_version', 'numeric')
                // ...
        },
    ])
;
```
As condition, for nested options you must to pass an array of values to resolve it on runtime, otherwise an exception will be thrown:
```php
$resolver->resolve(); // OK
$resolver->resolve(['database' => []]); // OK
$resolver->resolve(['database' => null); // KO (Exception!)
```
</details>

---

Demo app https://github.com/yceruto/nested-optionsresolver-demo

Commits
-------

d04e40b Added support for nested options definition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.