8000 [DI] Defaults to public=false in all service config files by nicolas-grekas · Pull Request #22615 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] Defaults to public=false in all service config files #22615

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.

Sign up for GitHub

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
May 3, 2017
Merged

[DI] Defaults to public=false in all service config files #22615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
[DI] Defaults to public=false in all service config files
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed May 3, 2017
commit 0656284f7fe49418f3417d9f2a996be8ee540603
15 changes: 8 additions & 7 deletions src/Symfony/Bundle/DebugBundle/Resources/config/services.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>
<service id="twig.extension.dump" class="Symfony\Bridge\Twig\Extension\DumpExtension" public="false">
<defaults public="false" />

<service id="twig.extension.dump" class="Symfony\Bridge\Twig\Extension\DumpExtension">
<tag name="twig.extension" />
<argument type="service" id="var_dumper.cloner" />
<argument type="service" id="var_dumper.html_dumper" />
</service>

<service id="data_collector.dump" class="Symfony\Component\HttpKernel\DataCollector\DumpDataCollector">
<service id="data_collector.dump" class="Symfony\Component\HttpKernel\DataCollector\DumpDataCollector" public="true">
<tag name="data_collector" id="dump" template="@Debug/Profiler/dump.html.twig" priority="240" />
<argument type="service" id="debug.stopwatch" on-invalid="ignore" />
<argument type="service" id="debug.file_link_formatter" on-invalid="ignore"></argument>
Expand All @@ -20,19 +22,19 @@
<argument>null</argument><!-- var_dumper.cli_dumper when debug.dump_destination is set -->
</service>

<service id="debug.dump_listener" class="Symfony\Component\HttpKernel\EventListener\DumpListener">
<service id="debug.dump_listener" class="Symfony\Component\HttpKernel\EventListener\DumpListener" public="true">
Copy link
Member

Choose a reason for hiding this comment

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

it would be great to be able to change this service to become private in 4.0 (as event listeners can now be private).
@nicolas-grekas I thnk a necessary feature for 3.4 would be a way to mark a service as "wanna-be-private", which would trigger a deprecation warning when getting it from the container through get, but not when injecting it inside another service (unlike deprecated services). Such feature would allow any bundle out there to provide gradual migration to private services (warning people if they try to access internal services that will become private)

Copy link
Member Author

Choose a reason for hiding this comment

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

100% agree, and this PR is on that path :)
No need for any new feature as there is already a trick to do the deprecation, see:

symfony/src/Symfony/Bundle/FrameworkBundle/Resources/config/form.xml

Line 192 in f2f9aa6

<service id="deprecated.form.registry" class="stdClass">

So, we should just turn these services as private, then register them in a public dummy registry, that will prevent inlining and will thus make them "public" (but in a deprecated manner).

Copy link
Member

Choose a reason for hiding this comment

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

no, the form types are deprecating the services entirely, because they won't be needed anymore in 4.0 (form types without any dependencies don't need to be registered explicitly anymore).

But we might have a case where we want to deprecate the runtime retrieval of services (to make them private) without deprecating the service itself (as you will want to keep it as a dependency of other services), wh F438 ich is what we need for listeners.

And making them public in a deprecated manner works for Symfony services in 3.4.x. but it does not work for the future of Symfony, because your proposal relies on the BC layer for accessing non-inlined private services. But Symfony 4 will not make such services public at all, even when not inlined, and so you cannot have a similar BC layer for the migration to private services.

Btw, if you want to be sure that the services stay public, you must create 2 dummy registries. If you create only one, the service may end up being inlined inside the dummy registry if other references get removed by some optimizations.

Copy link
Member Author
@nicolas-grekas nicolas-grekas May 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The form types I linked are the ones that are not deprecated because they need special instantiation.
You're right that this trick is valid only on 3.4. But maybe 3.4 is the last one that will have public services? So if using the trick is enough for 3.4, then maybe better not to work on a new feature for nothing?

Anyway, that's an interesting discussion, but should not be hidden in this PR :)

<tag name="kernel.event_subscriber" />
<argument type="service" id="var_dumper.cloner" />
<argument type="service" id="var_dumper.cli_dumper" />
</service>

<service id="var_dumper.cloner" class="Symfony\Component\VarDumper\Cloner\VarCloner" />
<service id="var_dumper.cli_dumper" class="Symfony\Component\VarDumper\Dumper\CliDumper">
<service id="var_dumper.cloner" class="Symfony\Component\VarDumper\Cloner\VarCloner" public="true" />
<service id="var_dumper.cli_dumper" class="Symfony\Component\VarDumper\Dumper\CliDumper" public="true">
<argument>null</argument><!-- debug.dump_destination -->
<argument>%kernel.charset%</argument>
<argument>0</argument> <!-- flags -->
</service>
<service id="var_dumper.html_dumper" class="Symfony\Component\VarDumper\Dumper\HtmlDumper" public="false">
<service id="var_dumper.html_dumper" class="Symfony\Component\VarDumper\Dumper\HtmlDumper">
<argument>null</argument>
<argument>%kernel.charset%</argument>
<argument>0</argument> <!-- flags -->
Expand All @@ -43,5 +45,4 @@
</call>
</service>
</services>

</container>
4 changes: 2 additions & 2 deletions src/Symfony/Bundle/DebugBundle/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
"symfony/var-dumper": "~2.8|~3.0"
},
"require-dev": {
"symfony/config": "~2.8|~3.0",
"symfony/dependency-injection": "~2.8|~3.0",
"symfony/config": "~3.3",
"symfony/dependency-injection": "~3.3",
"symfony/web-profiler-bundle": "~2.8|~3.0"
},
"suggest": {
Expand Down
16 changes: 9 additions & 7 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/annotations.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>
<service id="annotations.reader" class="Doctrine\Common\Annotations\AnnotationReader" public="false">
<defaults public="false" />

<service id="annotations.reader" class="Doctrine\Common\Annotations\AnnotationReader">
<call method="addGlobalIgnoredName">
<argument>required</argument>
<argument type="service">
Expand All @@ -18,27 +20,26 @@
</argument>
</call>
</service>
<service id="Doctrine\Common\Annotations\Reader" alias="annotations.reader" public="false" />

<service id="annotations.cached_reader" class="Doctrine\Common\Annotations\CachedReader" public="false">
<service id="annotations.cached_reader" class="Doctrine\Common\Annotations\CachedReader">
<argument type="service" id="annotations.reader" />
<argument type="service">
<service class="Doctrine\Common\Cache\ArrayCache" />
</argument>
<argument /><!-- Debug-Flag -->
</service>

<service id="annotations.filesystem_cache" class="Doctrine\Common\Cache\FilesystemCache" public="false">
<service id="annotations.filesystem_cache" class="Doctrine\Common\Cache\FilesystemCache">
<argument /><!-- Cache-Directory -->
</service>

<service id="annotations.cache_warmer" class="Symfony\Bundle\FrameworkBundle\CacheWarmer\AnnotationsCacheWarmer" public="false">
<service id="annotations.cache_warmer" class="Symfony\Bundle\FrameworkBundle\CacheWarmer\AnnotationsCacheWarmer">
<argument type="service" id="annotations.reader" />
<argument>%kernel.cache_dir%/annotations.php</argument>
<argument type="service" id="cache.annotations" />
</service>

<service id="annotations.cache" class="Symfony\Component\Cache\DoctrineProvider" public="false">
<service id="annotations.cache" class="Symfony\Component\Cache\DoctrineProvider">
<argument type="service">
<service class="Symfony\Component\Cache\Adapter\PhpArrayAdapter">
<factory class="Symfony\Component\Cache\Adapter\PhpArrayAdapter" method="create" />
Expand All @@ -48,6 +49,7 @@
</argument>
</service>

<service id="annotation_reader" alias="annotations.reader" />
<service id="annotation_reader" alias="annotations.reader" public="true" />
<service id="Doctrine\Common\Annotations\Reader" alias="annotation_reader" />
</services>
</container>
20 changes: 10 additions & 10 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/assets.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,43 +5,43 @@
xsi:schemaLocation="http:// F42D symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>
<defaults public="false" />

<service id="assets.packages" class="Symfony\Component\Asset\Packages">
<service id="assets.packages" class="Symfony\Component\Asset\Packages" public="true">
<argument type="service" id="assets.empty_package" /> <!-- default package -->
<argument type="collection" /> <!-- named packages -->
</service>
<service id="Symfony\Component\Asset\Packages" alias="assets.packages" public="false" />
<service id="Symfony\Component\Asset\Packages" alias="assets.packages" />

<service id="assets.empty_package" class="Symfony\Component\Asset\Package" public="false">
<service id="assets.empty_package" class="Symfony\Component\Asset\Package">
<argument type="service" id="assets.empty_version_strategy" />
</service>

<service id="assets.context" class="Symfony\Component\Asset\Context\RequestStackContext">
<service id="assets.context" class="Symfony\Component\Asset\Context\RequestStackContext" public="true">
<argument type="service" id="request_stack" />
</service>

<service id="assets.path_package" class="Symfony\Component\Asset\PathPackage" abstract="true">
<service id="assets.path_package" class="Symfony\Component\Asset\PathPackage" abstract="true" public="true">
<argument /> <!-- base path -->
<argument /> <!-- version strategy -->
<argument type="service" id="assets.context" />
</service>

<service id="assets.url_package" class="Symfony\Component\Asset\UrlPackage" abstract="true">
<service id="assets.url_package" class="Symfony\Component\Asset\UrlPackage" abstract="true" public="true">
<argument /> <!-- base URLs -->
<argument /> <!-- version strategy -->
<argument type="service" id="assets.context" />
</service>

<service id="assets.static_version_strategy" class="Symfony\Component\Asset\VersionStrategy\StaticVersionStrategy" abstract="true">
<service id="assets.static_version_strategy" class="Symfony\Component\Asset\VersionStrategy\StaticVersionStrategy" abstract="true" public="true">
<argument /> <!-- version -->
<argument /> <!-- format -->
</service>

<service id="assets.empty_version_strategy" class="Symfony\Component\Asset\VersionStrategy\EmptyVersionStrategy" public="false" />
<service id="assets.empty_version_strategy" class="Symfony\Component\Asset\VersionStrategy\EmptyVersionStrategy" />

<service id="assets.json_manifest_version_strategy" class="Symfony\Component\Asset\VersionStrategy\JsonManifestVersionStrategy" abstract="true" public="false">
<service id="assets.json_manifest_version_strategy" class="Symfony\Component\Asset\VersionStrategy\JsonManifestVersionStrategy" abstract="true">
<argument /> <!-- manifest path -->
</service>

</services>
</container>
28 changes: 14 additions & 14 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/cache.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>
<defaults public="false" />

<service id="cache.app" parent="cache.adapter.filesystem">
<service id="cache.app" parent="cache.adapter.filesystem" public="true">
<tag name="cache.pool" clearer="cache.app_clearer" />
</service>

<service id="cache.system" parent="cache.adapter.system">
<service id="cache.system" parent="cache.adapter.system" public="true">
<tag name="cache.pool" />
</service>

Expand All @@ -26,7 +27,7 @@
<tag name="cache.pool" />
</service>

<service id="cache.adapter.system" class="Symfony\Component\Cache\Adapter\AdapterInterface" abstract="true">
<service id="cache.adapter.system" class="Symfony\Component\Cache\Adapter\AdapterInterface" abstract="true" public="true">
<factory class="Symfony\Component\Cache\Adapter\AbstractAdapter" method="createSystemCache" />
<tag name="cache.pool" clearer="cache.default_clearer" />
<tag name="monolog.logger" channel="cache" />
Expand All @@ -37,7 +38,7 @@
<argument type="service" id="logger" on-invalid="ignore" />
</service>

<service id="cache.adapter.apcu" class="Symfony\Component\Cache\Adapter\ApcuAdapter" abstract="true">
<service id="cache.adapter.apcu" class="Symfony\Component\Cache\Adapter\ApcuAdapter" abstract="true" public="true">
<tag name="cache.pool" clearer="cache.default_clearer" />
<tag name="monolog.logger" channel="cache" />
<argument /> <!-- namespace -->
Expand All @@ -48,7 +49,7 @@
</call>
</service>

<service id="cache.adapter.doctrine" class="Symfony\Component\Cache\Adapter\DoctrineAdapter" abstract="true">
<service id="cache.adapter.doctrine" class="Symfony\Component\Cache\Adapter\DoctrineAdapter" abstract="true" public="true">
<tag name="cache.pool" provider="cache.default_doctrine_provider" clearer="cache.default_clearer" />
<tag name="monolog.logger" channel="cache" />
<argument /> <!-- Doctrine provider service -->
Expand All @@ -59,7 +60,7 @@
</call>
</service>

<service id="cache.adapter.filesystem" class="Symfony\Component\Cache\Adapter\FilesystemAdapter" abstract="true">
<service id="cache.adapter.filesystem" class="Symfony\Component\Cache\Adapter\FilesystemAdapter" abstract="true" public="true">
<tag name="cache.pool" clearer="cache.default_clearer" />
<tag name="monolog.logger" channel="cache" />
<argument /> <!-- namespace -->
Expand All @@ -70,14 +71,14 @@
</call>
</service>

<service id="cache.adapter.psr6" class="Symfony\Component\Cache\Adapter\ProxyAdapter" abstract="true">
<service id="cache.adapter.psr6" class="Symfony\Component\Cache\Adapter\ProxyAdapter" abstract="true" public="true">
<tag name="cache.pool" provider="cache.default_psr6_provider" clearer="cache.default_clearer" />
<argument /> <!-- PSR-6 provider service -->
<argument /> <!-- namespace -->
<argument>0</argument> <!-- default lifetime -->
</service>

<service id="cache.adapter.redis" class="Symfony\Component\Cache\Adapter\RedisAdapter" abstract="true">
<service id="cache.adapter.redis" class="Symfony\Component\Cache\Adapter\RedisAdapter" abstract="true" public="true">
<tag name="cache.pool" provider="cache.default_redis_provider" clearer="cache.default_clearer" />
<tag name="monolog.logger" channel="cache" />
<argument /> <!-- Redis connection service -->
Expand All @@ -88,7 +89,7 @@
</call>
</service>

<service id="cache.adapter.memcached" class="Symfony\Component\Cache\Adapter\MemcachedAdapter" abstract="true">
<service id="cache.adapter.memcached" class="Symfony\Component\Cache\Adapter\MemcachedAdapter" abstract="true" public="true">
<tag name="cache.pool" provider="cache.default_memcached_provider" clearer="cache.default_clearer" />
<tag name="monolog.logger" channel="cache" />
<argument /> <!-- Memcached connection service -->
Expand All @@ -99,13 +100,12 @@
</call>
</service>

<service id="cache.default_clearer" class="Symfony\Component\HttpKernel\CacheClearer\Psr6CacheClearer">
<service id="cache.default_clearer" class="Symfony\Component\HttpKernel\CacheClearer\Psr6CacheClearer" public="true">
<tag name="kernel.cache_clearer" />
</service>

<service id="cache.global_clearer" parent="cache.default_clearer" />
<service id="cache.app_clearer" alias="cache.default_clearer" />
<service id="Psr\Cache\CacheItemPoolInterface" alias="cache.app" public="false" />

<service id="cache.global_clearer" parent="cache.default_clearer" public="true" />
<service id="cache.app_clearer" alias="cache.default_clearer" public="true" />
<service id="Psr\Cache\CacheItemPoolInterface" alias="cache.app" />
</services>
</container>
2 changes: 2 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_debug.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>
<defaults public="false" />

<!-- DataCollector -->
<service id="data_collector.cache" class="Symfony\Component\Cache\DataCollector\CacheDataCollector">
Copy link
Member

Choose a reason for hiding this comment

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

this changes the visibility of this collector. Is it expected ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, the service does not need to be public

<tag name="data_collector" template="@WebProfiler/Collector/cache.html.twig" id="cache" priority="275" />
Expand Down
20 changes: 11 additions & 9 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/collectors.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,47 +5,49 @@
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>
<service id="data_collector.config" class="Symfony\Component\HttpKernel\DataCollector\ConfigDataCollector" public="false">
<defaults public="false" />

<service id="data_collector.config" class="Symfony\Component\HttpKernel\DataCollector\ConfigDataCollector">
<tag name="data_collector" template="@WebProfiler/Collector/config.html.twig" id="config" priority="-255" />
<call method="setKernel"><argument type="service" id="kernel" on-invalid="ignore" /></call>
</service>

<service id="data_collector.request" class="Symfony\Bundle\FrameworkBundle\DataCollector\RequestDataCollector">
<service id="data_collector.request" class="Symfony\Bundle\FrameworkBundle\DataCollector\RequestDataCollector" public="true">
<tag name="kernel.event_subscriber" />
<tag name="data_collector" template="@WebProfiler/Collector/request.html.twig" id="request" priority="335" />
</service>

<service id="data_collector.ajax" class="Symfony\Component\HttpKernel\DataCollector\AjaxDataCollector" public="false">
<service id="data_collector.ajax" class="Symfony\Component\HttpKernel\DataCollector\AjaxDataCollector">
<tag name="data_collector" template="@WebProfiler/Collector/ajax.html.twig" id="ajax" priority="315" />
</service>

<service id="data_collector.exception" class="Symfony\Component\HttpKernel\DataCollector\ExceptionDataCollector" public="false">
<service id="data_collector.exception" class="Symfony\Component\HttpKernel\DataCollector\ExceptionDataCollector">
<tag name="data_collector" template="@WebProfiler/Collector/exception.html.twig" id="exception" priority="305" />
</service>

<service id="data_collector.events" class="Symfony\Component\HttpKernel\DataCollector\EventDataCollector" public="false">
<service id="data_collector.events" class="Symfony\Component\HttpKernel\DataCollector\EventDataCollector">
<tag name="data_collector" template="@WebProfiler/Collector/events.html.twig" id="events" priority="290" />
<argument type="service" id="event_dispatcher" on-invalid="ignore" />
</service>

<service id="data_collector.logger" class="Symfony\Component\HttpKernel\DataCollector\LoggerDataCollector" public="false">
<service id="data_collector.logger" class="Symfony\Component\HttpKernel\DataCollector\LoggerDataCollector">
<tag name="data_collector" template="@WebProfiler/Collector/logger.html.twig" id="logger" priority="300" />
<tag name="monolog.logger" channel="profiler" />
<argument type="service" id="logger" on-invalid="ignore" />
<argument>%kernel.cache_dir%/%kernel.container_class%</argument>
</service>

<service id="data_collector.time" class="Symfony\Component\HttpKernel\DataCollector\TimeDataCollector" public="false">
<service id="data_collector.time" class="Symfony\Component\HttpKernel\DataCollector\TimeDataCollector">
<tag name="data_collector" template="@WebProfiler/Collector/time.html.twig" id="time" priority="330" />
<argument type="service" id="kernel" on-invalid="ignore" />
<argument type="service" id="debug.stopwatch" on-invalid="ignore" />
</service>

<service id="data_collector.memory" class="Symfony\Component\HttpKernel\DataCollector\MemoryDataCollector" public="false">
<service id="data_collector.memory" class="Symfony\Component\HttpKernel\DataCollector\MemoryDataCollector">
<tag name="data_collector" template="@WebProfiler/Collector/memory.html.twig" id="memory" priority="325" />
</service>

<service id="data_collector.router" class="Symfony\Bundle\FrameworkBundle\DataCollector\RouterDataCollector" >
<service id="data_collector.router" class="Symfony\Bundle\FrameworkBundle\DataCollector\RouterDataCollector" public="true">
<tag name="kernel.event_listener" event="kernel.controller" method="onKernelController"/>
<tag name="data_collector" template="@WebProfiler/Collector/router.html.twig" id="router" priority="285" />
</service>
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>
<defaults public="false" />

<service id="console.error_listener" class="Symfony\Component\Console\EventListener\ErrorListener" public="false">
<service id="console.error_listener" class="Symfony\Component\Console\EventListener\ErrorListener">
<argument type="service" id="logger" on-invalid="null" />
<tag name="kernel.event_subscriber" />
<tag name="monolog.logger" channel="console" />
</service>

</services>
</container>
Loading
0