Skip to content

[WIP] Use controller autowiring / DunglasActionBundle #315

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

Closed
wants to merge 7 commits into from
Closed

[WIP] Use controller autowiring / DunglasActionBundle #315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a9e1ebd
Use controller autowiring
dunglas Mar 20, 2016
2dbd8ab
Remove useless routing config
dunglas Mar 20, 2016
be48eb4
Add some autowired commands
dunglas Mar 21, 2016
539dfd8
Sync with symfony/symfony#960
dunglas Mar 25, 2016
35f1ea2
Remove some craps
dunglas Mar 25, 2016
6985cec
Update Travis config
dunglas Mar 26, 2016
436265d
Update to new DunglasActionBundle version
dunglas Mar 29, 2016
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/app/bootstrap.php.cache
/app/cache/*
/app/logs/*
/app/data/blog.sqlite
!app/cache/.gitkeep
!app/logs/.gitkeep
/build/
Expand Down
2 changes: 0 additions & 2 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ cache:
matrix:
fast_finish: true
include:
- php: 5.3
- php: 5.4
- php: 5.5
- php: 5.6
- php: 7.0
Expand Down
1 change: 1 addition & 0 deletions app/AppKernel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public function registerBundles()
new Symfony\Bundle\SwiftmailerBundle\SwiftmailerBundle(),
new Symfony\Bundle\AsseticBundle\AsseticBundle(),
new Doctrine\Bundle\DoctrineBundle\DoctrineBundle(),
new Dunglas\ActionBundle\DunglasActionBundle(),
new Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle(),
new Knp\Bundle\PaginatorBundle\KnpPaginatorBundle(),
new CodeExplorerBundle\CodeExplorerBundle(),
Expand Down
20 changes: 15 additions & 5 deletions app/SymfonyRequirements.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,11 +425,13 @@ public function __construct()
'Change the permissions of either "<strong>app/logs/</strong>" or "<strong>var/logs/</strong>" directory so that the web server can write into it.'
);

$this->addPhpIniRequirement(
'date.timezone', true, false,
'date.timezone setting must be set',
'Set the "<strong>date.timezone</strong>" setting in php.ini<a href="#phpini">*</a> (like Europe/Paris).'
);
if (version_compare($installedPhpVersion, '7.0.0', '<')) {
$this->addPhpIniRequirement(
'date.timezone', true, false,
'date.timezone setting must be set',
'Set the "<strong>date.timezone</strong>" setting in php.ini<a href="#phpini">*</a> (like Europe/Paris).'
);
}

if (version_compare($installedPhpVersion, self::REQUIRED_PHP_VERSION, '>=')) {
$timezones = array();
Expand Down Expand Up @@ -677,6 +679,14 @@ function_exists('posix_isatty'),
'Upgrade your <strong>intl</strong> extension with a newer ICU version (4+).'
);

if (class_exists('Symfony\Component\Intl\Intl')) {
$this->addRecommendation(
\Symfony\Component\Intl\Intl::getIcuDataVersion() === \Symfony\Component\Intl\Intl::getIcuVersion(),
sprintf('intl ICU version installed on your system (%s) should match the ICU data bundled with Symfony (%s)', \Symfony\Component\Intl\Intl::getIcuVersion(), \Symfony\Component\Intl\Intl::getIcuDataVersion()),
'In most cases you should be fine, but please verify there is no inconsistencies between data provided by Symfony and the intl extension. See https://github.com/symfony/symfony/issues/15007 for an example of inconsistencies you might run into.'
);
}

$this->addPhpIniRecommendation(
'intl.error_level',
create_function('$cfgValue', 'return (int) $cfgValue === 0;'),
Expand Down
1 change: 1 addition & 0 deletions app/config/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ framework:
form: ~
csrf_protection: ~
validation: { enable_annotations: true }
#serializer: { enable_annotations: true }
templating:
engines: ['twig']
default_locale: "%locale%"
Expand Down
20 changes: 11 additions & 9 deletions app/config/services.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
services:
# First we define some basic services to make these utilities available in
# the entire application
# Required for fixtures
slugger:
class: AppBundle\Utils\Slugger

markdown:
class: AppBundle\Utils\Markdown
class: AppBundle\Utils\Slugger
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the interesting part here.

Copy link
Member

Choose a reason for hiding this comment

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

In a real application, these services would be used in other services, in console commands, etc. You cannot remove them.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree than in a real app large app I would explicitly defined at least the slugger service. However, it's not necessary in this sample RAD app and it shows how the new controller system can be used (for RAD only of course).

Just some hints:

  • those services hey are used in other services here (@markdown is used in @app.twig.app_extension)
  • an advantage of using autowiring is if you change signature of the constructor of the AppExtension or if you rename the markdown service, you don't need to update your config
  • if the service using @markdown is itself autowired the @markdown is automatically injected (@app.twig.app_extension)
  • defining explicitly the service (adding back the @slugger definition for instance) doesn't break anything (the named service will be detected and injected everywhere the autowiring service is defined)
  • by convention, all services created by the autowiring system are accessible using the autowired.the\fqn name (but it's better to name them explicitly if you want to access them). Here there is a @autowired.AppBundle\Utils\Slugger service.

Btw, console commands can be autowired the same way than controller, I'm adding an example right now.


# 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%]
autowire: true
arguments:
# the first argument is autowired
1: %app_locales%
tags:
- { name: twig.extension }

Expand All @@ -23,8 +22,11 @@ services:
- { name: twig.extension }

app.redirect_to_preferred_locale_listener:
class: AppBundle\EventListener\RedirectToPreferredLocaleListener
arguments: ['@router', %app_locales%, %locale%]
class: AppBundle\EventListener\RedirectToPreferredLocaleListener
autowire: true
arguments:
1: %app_locales%
2: %locale%
tags:
- { name: kernel.event_listener, event: kernel.request, method: onKernelRequest }

Expand Down
Binary file removed app/data/blog.sqlite
View file
Open in desktop
Binary file not shown.
5 changes: 3 additions & 2 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
"symfony/monolog-bundle" : "~2.7",
"symfony/swiftmailer-bundle" : "~2.3",
"symfony/symfony" : "~2.8",
"twig/extensions" : "~1.2"
"twig/extensions" : "~1.2",
"dunglas/action-bundle" : "~1.0@dev"
},
"require-dev": {
"sensio/generator-bundle": "~3.0"
Expand All @@ -52,7 +53,7 @@
"config": {
"bin-dir": "bin",
"platform": {
Copy link
Member

Choose a reason for hiding this comment

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

not good. We want to keep the demo compatible with older PHP versions, to ensure that someone running symfony demo gets a working demo

@javiereguiluz we may switch to 5.4 as min target though, as the installer itself requires 5.4 anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same here. I'll restore it as is when it will be ready. But this PR will require Symfony 3.1 anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Well, this should be changed to 5.5.9 for the platform once the demo runs on Symfony 3.

"php": "5.3.9"
"php": "5.5.9"
}
},
"extra": {
Expand Down
Loading