Skip to content

[Config] Add docs for ConfigBuilders #15181

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
Apr 16, 2021

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Apr 2, 2021

This is config for symfony/symfony#40600

We should wait for the code merge

nicolas-grekas added a commit to symfony/symfony that referenced this pull request Apr 13, 2021
…r for writing PHP config (Nyholm)

This PR was squashed before being merged into the 5.3-dev branch.

Discussion
----------

[Config][DependencyInjection] Add configuration builder for writing PHP config

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        | symfony/symfony-docs#15181

I've spent most part of today to generate this PR. It is far from complete but it is ready for review.

This PR will build classes and store them in the build_dir. The classes will help you write PHP config. It will basically generate an array for you.

### Before

```php
// config/packages/security.php
<?php

use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $container) {
  $array = [
        'firewalls' => [
            'main' => [
                'pattern' => '^/*',
                'lazy' => true,
                'anonymous' => [],
            ],
        ],
        'access_control' => [
            [
                'path' => '^/user',
                'roles' => [
                    0 => 'ROLE_USER',
                ],
            ],
            [
                'path' => '^/admin',
                'roles' => 'ROLE_ADMIN',
            ],
        ],
        'role_hierarchy' => [
            'ROLE_ADMIN' => ['ROLE_USER'],
            'ROLE_SUPER_ADMIN' => ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH',
            ],
        ],
    ];

    $container->extension('security', $array);
}
```

### After
```php
// config/packages/security.php
<?php

use Symfony\Config\SecurityConfig;

return static function (SecurityConfig $security) {
    $security
        ->roleHierarchy('ROLE_ADMIN', ['ROLE_USER'])
        ->roleHierarchy('ROLE_SUPER_ADMIN', ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH'])
        ->accessControl()
            ->path('^/user')
            ->role('ROLE_USER');

    $security->accessControl(['path' => '^/admin', 'roles' => 'ROLE_ADMIN']);
    $security->firewall('main')
        ->pattern('^/*')
        ->lazy(true)
        ->anonymous();

};
```

### About autogeneration

This PR is generating the extension's `ConfigBuilder`s when they are first used. Since the PR is already very large, I prefer to follow up with additional PRs to include a cache warmer and command to rebuild the `ConfigBuilder`s.

The generated `ConfigBuilder` uses a "ucfirst() camelCased" extension alias. If the alias is `acme_foo` the root `ConfigBuilder` will be `Symfony\Config\AcmeFooConfig`.

The recommended way of using this class is:

```php
// config/packages/acme_foo.php
use Symfony\Config\AcmeFooConfig;

return static function (AcmeFooConfig $foo) {
  // ...
  // No need to return
}
```

One may also init the class directly, But this will not help you with generation or autoloading

```php
// config/packages/acme_foo.php
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $container) {
  $foo = new \Symfony\Config\AcmeFooConfig();
  // ...
  $container->extension('acme_foo', $foo->toArray());
}
```

**I do think we should only talk about the first way**

If a third party bundle like this idea and want to provide their own `ConfigBuilder`, they have two options:

1) Create the class `Symfony\Config\TheBundleConfig` themselves and make sure they configure composer to autoload that file and that the class implements `ConfigBuilderInterface`. We will never regenerate a file that already exists.

2) Create any class implementing `ConfigBuilderInterface` and ask their users to use that class in their config in the same way they would use `Symfony\Config\TheBundleConfig`.

The first way is obviously the smoothest way of doing things.

### BC

There is a great discussion about backwards compatibility for the generated files. We can assure that the class generator don't introduce a BC break with our tests. However, if the bundle changes their configuration definition it may break BC. Things like renaming, changing type or changing a single value to array is obvious BC breaks, however, these can be fixed in the config definition with normalisation.

The generator does not support normalisation. It is way way more complicated to reverse engineer that. I think a future update could fix this in one of two ways:
1) Add extra definition rules to help the class generator
2) Allow the bundle to patch part of the generated code

I hate BC breaks as much as the next person, but all the BC breaks in the generated classes will be caught when building the container (not at runtime), so I am fine with not having a 100% complete solution for this issue in the initial PR.

### Other limitations

If a bundle is using a custom extension alias, then we cannot guess it.. so a user have to use a cache warmer because we cannot generate the `ConfigBuilder` on the fly.

### TODO

- [x] Add tests
- [x] Update changelog
- [x] Write documentation

-------------

The generated code can be found in this example app: https://github.com/Nyholm/sf-issue-40600/tree/main/var/cache/dev/Symfony/Config

Commits
-------

460b46f [Config][DependencyInjection] Add configuration builder for writing PHP config
symfony-splitter pushed a commit to symfony/http-kernel that referenced this pull request Apr 13, 2021
…r for writing PHP config (Nyholm)

This PR was squashed before being merged into the 5.3-dev branch.

Discussion
----------

[Config][DependencyInjection] Add configuration builder for writing PHP config

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        | symfony/symfony-docs#15181

I've spent most part of today to generate this PR. It is far from complete but it is ready for review.

This PR will build classes and store them in the build_dir. The classes will help you write PHP config. It will basically generate an array for you.

### Before

```php
// config/packages/security.php
<?php

use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $container) {
  $array = [
        'firewalls' => [
            'main' => [
                'pattern' => '^/*',
                'lazy' => true,
                'anonymous' => [],
            ],
        ],
        'access_control' => [
            [
                'path' => '^/user',
                'roles' => [
                    0 => 'ROLE_USER',
                ],
            ],
            [
                'path' => '^/admin',
                'roles' => 'ROLE_ADMIN',
            ],
        ],
        'role_hierarchy' => [
            'ROLE_ADMIN' => ['ROLE_USER'],
            'ROLE_SUPER_ADMIN' => ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH',
            ],
        ],
    ];

    $container->extension('security', $array);
}
```

### After
```php
// config/packages/security.php
<?php

use Symfony\Config\SecurityConfig;

return static function (SecurityConfig $security) {
    $security
        ->roleHierarchy('ROLE_ADMIN', ['ROLE_USER'])
        ->roleHierarchy('ROLE_SUPER_ADMIN', ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH'])
        ->accessControl()
            ->path('^/user')
            ->role('ROLE_USER');

    $security->accessControl(['path' => '^/admin', 'roles' => 'ROLE_ADMIN']);
    $security->firewall('main')
        ->pattern('^/*')
        ->lazy(true)
        ->anonymous();

};
```

### About autogeneration

This PR is generating the extension's `ConfigBuilder`s when they are first used. Since the PR is already very large, I prefer to follow up with additional PRs to include a cache warmer and command to rebuild the `ConfigBuilder`s.

The generated `ConfigBuilder` uses a "ucfirst() camelCased" extension alias. If the alias is `acme_foo` the root `ConfigBuilder` will be `Symfony\Config\AcmeFooConfig`.

The recommended way of using this class is:

```php
// config/packages/acme_foo.php
use Symfony\Config\AcmeFooConfig;

return static function (AcmeFooConfig $foo) {
  // ...
  // No need to return
}
```

One may also init the class directly, But this will not help you with generation or autoloading

```php
// config/packages/acme_foo.php
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $container) {
  $foo = new \Symfony\Config\AcmeFooConfig();
  // ...
  $container->extension('acme_foo', $foo->toArray());
}
```

**I do think we should only talk about the first way**

If a third party bundle like this idea and want to provide their own `ConfigBuilder`, they have two options:

1) Create the class `Symfony\Config\TheBundleConfig` themselves and make sure they configure composer to autoload that file and that the class implements `ConfigBuilderInterface`. We will never regenerate a file that already exists.

2) Create any class implementing `ConfigBuilderInterface` and ask their users to use that class in their config in the same way they would use `Symfony\Config\TheBundleConfig`.

The first way is obviously the smoothest way of doing things.

### BC

There is a great discussion about backwards compatibility for the generated files. We can assure that the class generator don't introduce a BC break with our tests. However, if the bundle changes their configuration definition it may break BC. Things like renaming, changing type or changing a single value to array is obvious BC breaks, however, these can be fixed in the config definition with normalisation.

The generator does not support normalisation. It is way way more complicated to reverse engineer that. I think a future update could fix this in one of two ways:
1) Add extra definition rules to help the class generator
2) Allow the bundle to patch part of the generated code

I hate BC breaks as much as the next person, but all the BC breaks in the generated classes will be caught when building the container (not at runtime), so I am fine with not having a 100% complete solution for this issue in the initial PR.

### Other limitations

If a bundle is using a custom extension alias, then we cannot guess it.. so a user have to use a cache warmer because we cannot generate the `ConfigBuilder` on the fly.

### TODO

- [x] Add tests
- [x] Update changelog
- [x] Write documentation

-------------

The generated code can be found in this example app: https://github.com/Nyholm/sf-issue-40600/tree/main/var/cache/dev/Symfony/Config

Commits
-------

460b46f730 [Config][DependencyInjection] Add configuration builder for writing PHP config
symfony-splitter pushed a commit to symfony/dependency-injection that referenced this pull request Apr 13, 2021
…r for writing PHP config (Nyholm)

This PR was squashed before being merged into the 5.3-dev branch.

Discussion
----------

[Config][DependencyInjection] Add configuration builder for writing PHP config

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        | symfony/symfony-docs#15181

I've spent most part of today to generate this PR. It is far from complete but it is ready for review.

This PR will build classes and store them in the build_dir. The classes will help you write PHP config. It will basically generate an array for you.

### Before

```php
// config/packages/security.php
<?php

use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $container) {
  $array = [
        'firewalls' => [
            'main' => [
                'pattern' => '^/*',
                'lazy' => true,
                'anonymous' => [],
            ],
        ],
        'access_control' => [
            [
                'path' => '^/user',
                'roles' => [
                    0 => 'ROLE_USER',
                ],
            ],
            [
                'path' => '^/admin',
                'roles' => 'ROLE_ADMIN',
            ],
        ],
        'role_hierarchy' => [
            'ROLE_ADMIN' => ['ROLE_USER'],
            'ROLE_SUPER_ADMIN' => ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH',
            ],
        ],
    ];

    $container->extension('security', $array);
}
```

### After
```php
// config/packages/security.php
<?php

use Symfony\Config\SecurityConfig;

return static function (SecurityConfig $security) {
    $security
        ->roleHierarchy('ROLE_ADMIN', ['ROLE_USER'])
        ->roleHierarchy('ROLE_SUPER_ADMIN', ['ROLE_ADMIN', 'ROLE_ALLOWED_TO_SWITCH'])
        ->accessControl()
            ->path('^/user')
            ->role('ROLE_USER');

    $security->accessControl(['path' => '^/admin', 'roles' => 'ROLE_ADMIN']);
    $security->firewall('main')
        ->pattern('^/*')
        ->lazy(true)
        ->anonymous();

};
```

### About autogeneration

This PR is generating the extension's `ConfigBuilder`s when they are first used. Since the PR is already very large, I prefer to follow up with additional PRs to include a cache warmer and command to rebuild the `ConfigBuilder`s.

The generated `ConfigBuilder` uses a "ucfirst() camelCased" extension alias. If the alias is `acme_foo` the root `ConfigBuilder` will be `Symfony\Config\AcmeFooConfig`.

The recommended way of using this class is:

```php
// config/packages/acme_foo.php
use Symfony\Config\AcmeFooConfig;

return static function (AcmeFooConfig $foo) {
  // ...
  // No need to return
}
```

One may also init the class directly, But this will not help you with generation or autoloading

```php
// config/packages/acme_foo.php
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $container) {
  $foo = new \Symfony\Config\AcmeFooConfig();
  // ...
  $container->extension('acme_foo', $foo->toArray());
}
```

**I do think we should only talk about the first way**

If a third party bundle like this idea and want to provide their own `ConfigBuilder`, they have two options:

1) Create the class `Symfony\Config\TheBundleConfig` themselves and make sure they configure composer to autoload that file and that the class implements `ConfigBuilderInterface`. We will never regenerate a file that already exists.

2) Create any class implementing `ConfigBuilderInterface` and ask their users to use that class in their config in the same way they would use `Symfony\Config\TheBundleConfig`.

The first way is obviously the smoothest way of doing things.

### BC

There is a great discussion about backwards compatibility for the generated files. We can assure that the class generator don't introduce a BC break with our tests. However, if the bundle changes their configuration definition it may break BC. Things like renaming, changing type or changing a single value to array is obvious BC breaks, however, these can be fixed in the config definition with normalisation.

The generator does not support normalisation. It is way way more complicated to reverse engineer that. I think a future update could fix this in one of two ways:
1) Add extra definition rules to help the class generator
2) Allow the bundle to patch part of the generated code

I hate BC breaks as much as the next person, but all the BC breaks in the generated classes will be caught when building the container (not at runtime), so I am fine with not having a 100% complete solution for this issue in the initial PR.

### Other limitations

If a bundle is using a custom extension alias, then we cannot guess it.. so a user have to use a cache warmer because we cannot generate the `ConfigBuilder` on the fly.

### TODO

- [x] Add tests
- [x] Update changelog
- [x] Write documentation

-------------

The generated code can be found in this example app: https://github.com/Nyholm/sf-issue-40600/tree/main/var/cache/dev/Symfony/Config

Commits
-------

460b46f730 [Config][DependencyInjection] Add configuration builder for writing PHP config
@Nyholm
Copy link
Member Author

Nyholm commented Apr 13, 2021

Wohoo. The code PR is merged.

@wouterj wouterj removed the Waiting Code Merge Docs for features pending to be merged label Apr 13, 2021
@Nyholm
Copy link
Member Author

Nyholm commented Apr 16, 2021

CI is green

@javiereguiluz javiereguiluz force-pushed the config-config-bulders branch from ee48d50 to 5b427af Compare April 16, 2021 13:57
@javiereguiluz javiereguiluz merged commit 9b9535d into symfony:5.x Apr 16, 2021
@Nyholm Nyholm deleted the config-config-bulders branch April 16, 2021 14:01
@Nyholm
Copy link
Member Author

Nyholm commented Apr 16, 2021

Thank you for merging.

@javiereguiluz
Copy link
Member

This is now merged (we tweaked it a bit while merging to add a link to the kernel.build_dir option explanation). Tobias, I haven't tested this yet but I sense this is going to be amazing. Thanks for contributing this feature and for providing its docs too.

@wouterj
Copy link
Member

wouterj commented Apr 16, 2021

@javiereguiluz what do you think about my (now hidden) comment:

However: why still document the old array syntax? If this PR is accepted for 5.3, I think we should update all PHP array examples to use this instead of arrays (just like we did with the DI and Routing configurators).

@javiereguiluz
Copy link
Member

Sorry I missed it! Yes, I agree with you. "PHP arrays" and "PHP ConfigBuilders" achieve the same, but builders look much much better, so why document both? Better use only the new and better configuration.

@Nyholm
Copy link
Member Author

Nyholm commented Apr 16, 2021

I'll prepare some PRs.

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.

6 participants