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

[FrameworkBundle] Show injected services for iterator and array arguments #31353

Conversation

Projects
None yet
5 participants
@jschaedl
Copy link
Contributor

commented May 1, 2019

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

When I have the following service configuration:

    App\Word\Checker\StaticWordChecker:
        tags: [app.checker]

    App\Word\Checker\BannedWorldListChecker:
        tags: [app.checker]

    App\Word\WordCheckerTaggedIterator:
        arguments: [!tagged app.checker]

    App\Word\WordCheckerArray:
        arguments:
            - App\Word\Checker\StaticWordChecker: ~
              App\Word\Checker\BannedWorldListChecker: ~

and I run:
./bin/console debug:container App\Word\WordCheckerArray --show-arguments

Information for Service "App\Word\WordCheckerArray"
===================================================

 ---------------- -------------------------------------------
  Option           Value
 ---------------- -------------------------------------------
  Service ID       App\Word\WordCheckerArray
  Class            App\Word\WordCheckerArray
  Tags             -
  Public           no
  Synthetic        no
  Lazy             no
  Shared           yes
  Abstract         no
  Autowired        yes
  Autoconfigured   yes
  Arguments        Array (2 element(s))
                   - App\Word\Checker\StaticWordChecker
                   - App\Word\Checker\BannedWorldListChecker
 ---------------- -------------------------------------------

or

./bin/console debug:container App\Word\WordCheckerTaggedIterator --show-arguments

Information for Service "App\Word\WordCheckerTaggedIterator"
============================================================

 ---------------- -------------------------------------------
  Option           Value
 ---------------- -------------------------------------------
  Service ID       App\Word\WordCheckerTaggedIterator
  Class            App\Word\WordCheckerTaggedIterator
  Tags             -
  Public           no
  Synthetic        no
  Lazy             no
  Shared           yes
  Abstract         no
  Autowired        yes
  Autoconfigured   yes
  Arguments        Iterator (2 element(s))
                   - App\Word\Checker\BannedWorldListChecker
                   - App\Word\Checker\StaticWordChecker
 ---------------- -------------------------------------------

I can now see the the objects injected into the iterator and array arguments.

@jschaedl jschaedl force-pushed the jschaedl:feature-31340_enhanced-information-debug-container-command branch from 2781480 to 21f7c9a May 1, 2019

@linaori

This comment has been minimized.

Copy link
Contributor

commented May 1, 2019

Should this be done on 3.4 instead? Currently 3.4 shows 0 instead of X in the iterator. This PR can be seen as a bug fix imo

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented May 1, 2019

We're too lax currently merging features as bug fixes, introducing regressions actually. Let's be a bit stricter, this is for master to me too.

@nicolas-grekas nicolas-grekas added this to the next milestone May 1, 2019

@linaori

This comment has been minimized.

Copy link
Contributor

commented May 2, 2019

@nicolas-grekas for the value display, I can agree. The fact that this iterator is showing 0 instead of the actual amount is the initial bug I reported. Perhaps the PR could be split up to put the count fix in 3.4 and the list of services in the master?

@jschaedl

This comment has been minimized.

Copy link
Contributor Author

commented May 2, 2019

@linaori The fact that this iterator is showing 0 in branch 3.4 is caused by the usage of a non compiled container in the ContainerDebugCommand (see: https://github.com/symfony/symfony/blob/3.4/src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php#L184) as far as I can see it. Therefore the service definitions for the iterable arguments are not resolved and the TaggedIteratorArgument::getValues() is returning an empty array which leads to showing 0.

@nicolas-grekas Using a compiled container in the DebugContainerCommand was introduced in #27684 I think to fix this 0 issue, we need to do the same in the for 3.4.

@jschaedl jschaedl force-pushed the jschaedl:feature-31340_enhanced-information-debug-container-command branch from cc3c5d2 to db5fb20 May 5, 2019

@fabpot

fabpot approved these changes May 6, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented May 6, 2019

Thank you @jschaedl.

@fabpot fabpot merged commit db5fb20 into symfony:master May 6, 2019

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request May 6, 2019

feature #31353 [FrameworkBundle] Show injected services for iterator …
…and array arguments (jschaedl)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[FrameworkBundle] Show injected services for iterator and array arguments

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      |no
| New feature?  | yes<!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| 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 | #31340   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | tbd.

When I have the following service configuration:

```yaml
    App\Word\Checker\StaticWordChecker:
        tags: [app.checker]

    App\Word\Checker\BannedWorldListChecker:
        tags: [app.checker]

    App\Word\WordCheckerTaggedIterator:
        arguments: [!tagged app.checker]

    App\Word\WordCheckerArray:
        arguments:
            - App\Word\Checker\StaticWordChecker: ~
              App\Word\Checker\BannedWorldListChecker: ~
```

and I run:
`./bin/console debug:container App\Word\WordCheckerArray --show-arguments`
```bash
Information for Service "App\Word\WordCheckerArray"
===================================================

 ---------------- -------------------------------------------
  Option           Value
 ---------------- -------------------------------------------
  Service ID       App\Word\WordCheckerArray
  Class            App\Word\WordCheckerArray
  Tags             -
  Public           no
  Synthetic        no
  Lazy             no
  Shared           yes
  Abstract         no
  Autowired        yes
  Autoconfigured   yes
  Arguments        Array (2 element(s))
                   - App\Word\Checker\StaticWordChecker
                   - App\Word\Checker\BannedWorldListChecker
 ---------------- -------------------------------------------
```
or

`./bin/console debug:container App\Word\WordCheckerTaggedIterator --show-arguments`
```bash
Information for Service "App\Word\WordCheckerTaggedIterator"
============================================================

 ---------------- -------------------------------------------
  Option           Value
 ---------------- -------------------------------------------
  Service ID       App\Word\WordCheckerTaggedIterator
  Class            App\Word\WordCheckerTaggedIterator
  Tags             -
  Public           no
  Synthetic        no
  Lazy             no
  Shared           yes
  Abstract         no
  Autowired        yes
  Autoconfigured   yes
  Arguments        Iterator (2 element(s))
                   - App\Word\Checker\BannedWorldListChecker
                   - App\Word\Checker\StaticWordChecker
 ---------------- -------------------------------------------
```

I can now see the the objects injected into the iterator and array arguments.

Commits
-------

db5fb20 [FrameworkBundle] Show injected services for Iterator and Array

@fabpot fabpot referenced this pull request May 9, 2019

Merged

Release v4.3.0-BETA1 #31435

} elseif ($argument instanceof ServiceLocatorArgument) {
$argumentsInformation[] = sprintf('Service locator (%d element(s))', \count($argument->getValues()));
} elseif ($argument instanceof Definition) {
$argumentsInformation[] = 'Inlined Service';
} else {
$argumentsInformation[] = \is_array($argument) ? sprintf('Array (%d element(s))', \count($argument)) : $argument;
if (\is_array($argument)) {
foreach (array_keys($argument) as $service) {
$argumentsInformation[] = sprintf('- %s', $service);

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas May 9, 2019

Member

I reverted this part in 7a53e8d as I don't see how listing keys can be useful.

@@ -348,12 +348,20 @@ protected function describeContainerDefinition(Definition $definition, array $op
$argumentsInformation[] = sprintf('Service(%s)', (string) $argument);
} elseif ($argument instanceof IteratorArgument) {
$argumentsInformation[] = sprintf('Iterator (%d element(s))', \count($argument->getValues()));
foreach (array_map(function (Reference $value) {return (string) $value; }, $argument->getValues()) as $service) {
$argumentsInformation[] = sprintf('- %s', $service);

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas May 9, 2019

Member

since this is a reference, it should be displayed as Service(%s)
fixed in 7a53e8d too

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.