Navigation Menu

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

[DI] Add Yaml syntax for short services definition #21313

Merged
merged 1 commit into from Jan 23, 2017
Merged

[DI] Add Yaml syntax for short services definition #21313

merged 1 commit into from Jan 23, 2017

Conversation

ogizanagi
Copy link
Member

@ogizanagi ogizanagi commented Jan 16, 2017

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR todo

In my experience, most (at least, a lot) of the service definitions in an end-application only require the class and constructor arguments.

#21133 allows to get rid of the class attribute by using the service id.
Which means only arguments remain for most use-cases. Hence, we could support this syntax:

Before:

services:
    App\Foo\Bar: 
        arguments: ['@baz', 'foo', '%qux%']

After:

services:
    App\Foo\Bar: ['@baz', 'foo', '%qux%']

It works especially well along with services _defaults introduced in #21071 :

services:
    _defaults:
        public: false
        autowire: ['set*']
        tags: ['serializer.normalizer']

    App\Serializer\FooNormalizer: ['@baz', 'foo', '%qux%']

@javiereguiluz
Copy link
Member

👍 I like this proposal a lot ... my only minor fear is that we may be introducing too many different ways/shortcuts to configure things.

@ogizanagi
Copy link
Member Author

ogizanagi commented Jan 20, 2017

I understand your fear, but to advocate even more this one, I find this way very intuitive as it echoes a php object instantiation:

App\Serializer\FooNormalizer: ['@baz', 'foo', '%qux%']
new \App\Serializer\FooNormalizer(new Baz(), 'foo', 'qux_value')

@@ -229,6 +229,10 @@ private function parseDefinition($id, $service, $file, array $defaults)
return;
}

if (is_array($service) && array_keys($service) === range(0, count($service) - 1)) {
Copy link
Member

Choose a reason for hiding this comment

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

array_values($service) === $service?

@dunglas
Copy link
Member

dunglas commented Jan 23, 2017

👍

@fabpot fabpot merged commit 83b599c into symfony:master Jan 23, 2017
fabpot added a commit that referenced this pull request Jan 23, 2017
…izanagi)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Add Yaml syntax for short services definition

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | todo

In my experience, most (at least, a lot) of the service definitions in an end-application only require the class and constructor arguments.

#21133 allows to get rid of the `class` attribute by using the service id.
Which means only arguments remain for most use-cases. Hence, we could support this syntax:

#### Before:

```yml
services:
    App\Foo\Bar:
        arguments: ['@baz', 'foo', '%qux%']
```

#### After:

```yml
services:
    App\Foo\Bar: ['@baz', 'foo', '%qux%']
```

It works especially well along with services `_defaults` introduced in #21071 :

```yml
services:
    _defaults:
        public: false
        autowire: ['set*']
        tags: ['serializer.normalizer']

    App\Serializer\FooNormalizer: ['@baz', 'foo', '%qux%']
```

Commits
-------

83b599c [DI] Add Yaml syntax for short services definition
@ogizanagi ogizanagi deleted the di/yaml_short_service_definition branch January 23, 2017 21:44
@fabpot fabpot mentioned this pull request May 1, 2017
@TomasVotruba
Copy link
Contributor

TomasVotruba commented Mar 9, 2018

Awesome feature 👍

Is there something similar for method calls?

I'd like to use it for service definitions

@xabbuh
Copy link
Member

xabbuh commented Mar 10, 2018

services:
    App\Foo:
        calls:
            - [method, [arg1, arg2]]

You mean this?

@TomasVotruba
Copy link
Contributor

TomasVotruba commented Mar 10, 2018

Yep, just smaller.

I have following use case - I need to merge 3rd party classes in simple style in services:

services:
    App\Foo:
        key: value

But in the background (custom YamlFileLoader?) I need to covert to 2 types based on 2 interfaces they can implement:

One to:

services:
    App\FooTypeA:
        properties:
            key: value

and other type:

services:
    App\FooTypeB:
        calls:
            - ['setKey', ['value']]

Key is to keep configuration simple and make use of services (for various reasons).

Any ideas for this?

@TomasVotruba
Copy link
Contributor

TomasVotruba commented Mar 10, 2018

After many Locator, Provider, YamlFileLoader, cusom file parsing etc. I came to this solution:

https://github.com/Symplify/Symplify/blob/d27851e30db419b1960f7b98e7d91a62fbd7cb7b/packages/EasyCodingStandard/src/Yaml/CheckerTolerantYamlFileLoader.php#L27-L81

With all the troubles I'm super happy. But any ideas are welcomed :)

<?php declare(strict_types=1);

namespace Symplify\EasyCodingStandard\Yaml;

use Nette\Utils\Strings;
use Symfony\Component\Config\FileLocatorInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;

/**
 * The need: https://github.com/symfony/symfony/pull/21313#issuecomment-372037445
 */
final class CheckerTolerantYamlFileLoader extends YamlFileLoader
{
    /**
     * @var CheckerConfigurationGuardian
     */
    private $checkerConfigurationGuardian;

    public function __construct(ContainerBuilder $containerBuilder, FileLocatorInterface $fileLocator)
    {
        $this->checkerConfigurationGuardian = new CheckerConfigurationGuardian();

        parent::__construct($containerBuilder, $fileLocator);
    }

    /**
     * @param string $file
     * @return array|mixed|mixed[]
     */
    protected function loadFile($file)
    {
        $decodedYaml = parent::loadFile($file);

        if (isset($decodedYaml['services'])) {
            return $this->moveArgumentsToPropertiesOrMethodCalls($decodedYaml);
        }

        return $decodedYaml;
    }

    /**
     * @param mixed[] $yaml
     * @return mixed[]
     */
    private function moveArgumentsToPropertiesOrMethodCalls(array $yaml): array
    {
        foreach ($yaml['services'] as $checker => $serviceDefinition) {
            if (empty($serviceDefinition)) {
                continue;
            }

            // is checker service?
            if (! Strings::endsWith($checker, 'Fixer') && ! Strings::endsWith($checker, 'Sniff')) {
                continue;
            }

            if (Strings::endsWith($checker, 'Fixer')) {
                $this->checkerConfigurationGuardian->ensureFixerIsConfigurable($checker, $serviceDefinition);
                // move parameters to "configure()" call
                $yaml['services'][$checker]['calls'] = [
                    ['configure', [$serviceDefinition]],
                ];
            }

            if (Strings::endsWith($checker, 'Sniff')) {
                // move parameters to property setters
                foreach ($serviceDefinition as $key => $value) {
                    $this->checkerConfigurationGuardian->ensurePropertyExists($checker, $key);
                    $yaml['services'][$checker]['properties'][$key] = $this->escapeValue($value);
                }
            }

            // cleanup parameters
            foreach ($serviceDefinition as $key => $value) {
                unset($yaml['services'][$checker][$key]);
            }
        }

        return $yaml;
    }

    /**
     * @param mixed $value
     * @return mixed
     */
    private function escapeValue($value)
    {
        if (is_numeric($value)) {
            return $value;
        }

        return Strings::replace($value, '#@#', '@@');
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants