Skip to content

[WIP] make the app/config/services.yml file use the new Symfony autoregistration and autowiring features. #483

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
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion app/AppKernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ public function registerBundles()
$bundles[] = new Symfony\Bundle\DebugBundle\DebugBundle();
$bundles[] = new Symfony\Bundle\WebProfilerBundle\WebProfilerBundle();
$bundles[] = new Sensio\Bundle\DistributionBundle\SensioDistributionBundle();
$bundles[] = new Sensio\Bundle\GeneratorBundle\SensioGeneratorBundle();

if ('dev' === $this->getEnvironment()) {
$bundles[] = new Symfony\Bundle\WebServerBundle\WebServerBundle();
$bundles[] = new Sensio\Bundle\GeneratorBundle\SensioGeneratorBundle();
}

if ('test' === $this->getEnvironment()) {
// this bundle makes it easier to work with databases in PHPUnit
Expand Down
172 changes: 102 additions & 70 deletions app/config/services.yml
Original file line number Diff line number Diff line change
@@ -1,83 +1,115 @@
services:
# First we define some basic services to make these utilities available in
# the entire application
slugger:
class: AppBundle\Utils\Slugger

markdown:
class: AppBundle\Utils\Markdown
# The special "_defaults" section configures the default behaviors for registering
# new services into the Symfony service container. This section has a local scope
# only, which means it only affects the service definitions being registered by
# this file.
#
# The "autowire" section tells Symfony to make its best to autowire new services
# by introspecting their constructor.
#
# The "public" section with the FALSE boolean value forces all new autowired classes
# to be declared as private services. Marking services private limits their scope to
# the Symfony service container itself only. They're not accessible from the outside
# world and thus let the service container optimizes itself when it's being compiled
# and dumped to raw PHP code.
_defaults:
autowire: true
public: false
Copy link
Member

Choose a reason for hiding this comment

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

Add the new autoconfigure: true


# These are the Twig extensions that create new filters and functions for
# using them in the templates
app.twig.app_extension:
public: false
class: AppBundle\Twig\AppExtension
arguments: ['@markdown', '%app_locales%']
tags:
- { name: twig.extension }
# The special "_instanceof" section appends additional wiring configuration
# to any service definitions that match the listed types.
#
# For instance, the first rule adds a special "kernel.event_subscriber" tag
# to any definitions declared in this file whose class is an instance of the
# "Symfony\Component\EventDispatcher\EventSubscriberInterface" class or interface.
_instanceof:
Symfony\Component\EventDispatcher\EventSubscriberInterface:
tags: ['kernel.event_subscriber']

app.twig.intl_extension:
public: false
class: Twig_Extensions_Extension_Intl
tags:
- { name: twig.extension }
Symfony\Component\Form\FormTypeInterface:
tags: ['form.type']

# Defining a form type as a service is only required when the form type
# needs to use some other services, such as the entity manager.
# See http://symfony.com/doc/current/best_practices/forms.html
app.form.type.tagsinput:
class: AppBundle\Form\Type\TagsInputType
arguments: ['@doctrine.orm.entity_manager']
tags:
- { name: form.type }
Symfony\Component\Security\Core\Authorization\VoterInterface:
tags: ['security.voter']

# Event Listeners are classes that listen to one or more specific events.
# Those events are defined in the tags added to the service definition.
# See http://symfony.com/doc/current/event_dispatcher.html#creating-an-event-listener
app.redirect_to_preferred_locale_listener:
class: AppBundle\EventListener\RedirectToPreferredLocaleListener
arguments: ['@router', '%app_locales%', '%locale%']
tags:
- { name: kernel.event_listener, event: kernel.request, method: onKernelRequest }
Twig_ExtensionInterface:
tags: ['twig.extension']
Copy link
Member

Choose a reason for hiding this comment

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

All of these _instanceof can be removed thanks to autoconfigure - I don't think we'll need any _instanceof in this file.


app.comment_notification:
class: AppBundle\EventListener\CommentNotificationListener
arguments: ['@mailer', '@router', '@translator', '%app.notifications.email_sender%']
# The "method" attribute of this tag is optional and defaults to "on + camelCasedEventName"
# If the event is "comment.created" the method executed by default is "onCommentCreated()".
tags:
- { name: kernel.event_listener, event: comment.created, method: onCommentCreated }
# The following settings will be appended to any local service definitions
# whose class matches this type. In this example, it will only apply to
# the "AppBundle\Controller\Admin\BlogController" service discovered by
# the "AppBundle\Controller\" rule below.
AppBundle\Controller\Admin\BlogController:
getters:
getSlugger: '@AppBundle\Utils\Slugger'

# Event subscribers are similar to event listeners but they don't need service tags.
# Instead, the PHP class of the event subscriber includes a method that returns
# the list of events listened by that class.
# See http://symfony.com/doc/current/event_dispatcher.html#creating-an-event-subscriber
app.requirements_subscriber:
class: AppBundle\EventListener\CheckRequirementsSubscriber
arguments: ['@doctrine.orm.entity_manager']
tags:
- { name: kernel.event_subscriber }
# This section enables to automatically register all classes found in the matching
# file paths and directories as services in the container. File and directory
# matching uses any valid glob pattern to create a white list of paths.
#
# In this example, the classes found in the following directories will be
# automatically registered as services:
#
# * src/AppBundle/EventListener/
# * src/AppBundle/Form/Type/
# * src/AppBundle/Security/
# * src/AppBundle/Twig/
# * src/AppBundle/Utils/
#
# The "resource" attribute can accept more specific glob patterns like
# "{EventListener/*Subscriber.php,Form/Type/*Type.php}".
AppBundle\:
# Register all classes in the src/AppBundle directory as services
resource: '../../src/AppBundle/{EventListener,Form/Type,Security,Twig,Utils}'
Copy link
Member

Choose a reason for hiding this comment

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

most form types don't need to be registered as services at all (they need to become services only when they have dependencies).
And as this is broken currently because you define private services, you could avoid doing it for all of them

Copy link
Author

Choose a reason for hiding this comment

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

Yes the current implementation is broken. I'm working on a PR to make form type services privatable.

Copy link
Member

Choose a reason for hiding this comment

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

If symfony/symfony-standard#1070 is merged, this will simplify


# To inject the voter into the security layer, you must declare it as a service and tag it with security.voter.
# See http://symfony.com/doc/current/security/voters.html#configuring-the-voter
app.post_voter:
class: AppBundle\Security\PostVoter
public: false
tags:
- { name: security.voter }
# The other section defines a rule to automatically register and autowire the
# controller classes found in the src/AppBundle/Controller/ directory.
#
# By default all services are made private according to the global "_defaults"
# section at the top of this file.
#
# However, in Symfony, controllers must always be declared public in order to
# be lazy instanciated when they're really needed. This is why the inherited
# default "public" attribute is overriden to force the registered controller
# services to be marked public.
AppBundle\Controller\:
Copy link
Member

Choose a reason for hiding this comment

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

WYDT about removing the AppBundle like in symfony/symfony-standard#1034?

Copy link
Author

Choose a reason for hiding this comment

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

Interesting discussion. I really like the idea of getting rid of the AppBundle.

Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry but we are not going to get rid of AppBundle in the Symfony Demo. For this kind of changes, the Symfony Demo must go behind the Symfony Framework itself. If Symfony decides to go bundle-less for Symfony 4.0 (I wish it will) then we'll update the Symfony Demo accordingly. But we can't do that before. I'm sorry.

Copy link
Member

Choose a reason for hiding this comment

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

We can still create PR to test this setup without merging it.

resource: '../../src/AppBundle/Controller'
public: true

# Uncomment the following lines to define a service for the Post Doctrine repository.
# It's not mandatory to create these services, but if you use repositories a lot,
# these services simplify your code:
# This third party Twig extension must be manually registered as a service
# because its class doesn't live in any of the previous defined directories.
#
# app.post_repository:
# class: Doctrine\ORM\EntityRepository
# factory: ['@doctrine.orm.entity_manager', getRepository]
# arguments: [AppBundle\Entity\Post]
# Thus, its class is not namespaced and cannot be defined in a global rule.
# Indeed, registering new classes whose filename matches a glob pattern as
# services like in the two previous sections only works for namespaced classes.
app.twig.intl_extension:
Copy link
Member

Choose a reason for hiding this comment

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

Would we want this to also match the class name?

Copy link
Member

Choose a reason for hiding this comment

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

Yea, I think it should be:

# register vendor class a service
Twig_Extensions_Extension_Intl: ~

Copy link
Member

Choose a reason for hiding this comment

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

If I do that, I get this error:

RuntimeException in CheckDefinitionValidityPass.php line 52:

The definition for "Twig_Extensions_Extension_Intl" has no class attribute, and
appears to reference a class or interface in the global namespace. Leaving out
the "class" attribute is only allowed for namespaced classes. Please specify the
class attribute explicitly to get rid of this error.

class: Twig_Extensions_Extension_Intl

# Some classes cannot be fully autowired because their methods accept either
# some scalar arguments that Symfony cannot guess or a typehinted dependency
# for which the container has at least two registered services matching the
# type.
#
# Both the "AppBundle\Twig\AppExtension" and "AppBundle\EventListener\RedirectToPreferredLocaleSubscriber"
# classes have a "__construct()" method that receives a "$locales" scalar argument
# that Symfony cannot guess. This is why we must manually and explicitly provide
# the wiring of this argument to complete their service definitions.
#
# // traditional code inside a controller
# $entityManager = $this->getDoctrine()->getManager();
# $posts = $entityManager->getRepository('AppBundle:Post')->findAll();
# To do so, the remaining unwired named arguments must be defined with their
# corresponding values. Here, the "$locales" named argument is configured to
# receive the value of the global "%app_locales%" parameter defined under the
# "parameters" section of the "app/config/config.yml" file.
#
# // same code using repository services
# $posts = $this->get('app.post_repository')->findAll();
# Note that the order in which the named arguments are defined below doesn't
# matter as they're referenced here by their real names in the PHP code.
# Symfony is then smart enough to make the corresponding matching when compiling,
# optimizing and dumping the container as a raw PHP class in the cache directory.
AppBundle\Twig\AppExtension:
Copy link
Member

Choose a reason for hiding this comment

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

This may introduce some confusion. The reason is that you already configured this service previously as part of:

AppBundle\:
  resource: '../../src/AppBundle/{EventListener/*Subscriber.php,Form/Type,Security,Twig,Utils}'

And also:

_instanceof:
  Twig_ExtensionInterface:
    tags: ['twig.extension']

And now you are partially configuring it ... or completing its configuration:

AppBundle\Twig\AppExtension:
  $locales: '%app_locales%'

So ... the AppBundle\Twig\AppExtension is defined in three different parts of the file 😕

Copy link
Author

Choose a reason for hiding this comment

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

The first section lets Symfony register the classes from these directories as services. It'll make its best to autowire everything. But some service requires some extra manual configuration when Symfony cannot guess the remaining parts (dependency collision or scalar arguments). This is what the third section does.

The second section called instanceof allows to append configuration to every definitions that match the rule type. It has nothing to do with autowiring! This is probably the confusing part that must be well taught to newcomers. It's just a way to apply a group of configuration settings to each service definition matching the rule. It's a way to make configuration not repeat itself. This means you can still manually define your services and use the instanceof section to append the DI tags or extra method calls to your sets of configured services.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, @hhamon I didn't know about _instanceof config section. Are there any docs about it? Could be appended to comment to let people know what's the magic behind it.

Copy link
Author

Choose a reason for hiding this comment

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

Documentation doesn't exist yet. This PR is a PoC to experiment the upcoming new DI features of Symfony 3.3. We'll document everything very soon ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I've searched for _instance here and it was only in config so I thought it is already supported elsewhere.

Copy link
Member

Choose a reason for hiding this comment

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

This part - where we fill in missing arguments - should stay as it is - this is perfect

$locales: '%app_locales%'

AppBundle\EventListener\RedirectToPreferredLocaleSubscriber:
$locales: '%app_locales%'

AppBundle\EventListener\CommentNotificationSubscriber:
$sender: '%app.notifications.email_sender%'
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"symfony/monolog-bundle" : "^3.0",
"symfony/polyfill-apcu" : "^1.0",
"symfony/swiftmailer-bundle" : "^2.3",
"symfony/symfony" : "^3.2",
"symfony/symfony" : "^3.3@dev",
"twig/extensions" : "^1.3",
"twig/twig" : "^1.28 || ^2.0",
"white-october/pagerfanta-bundle" : "^1.0"
Expand Down
Loading