merged branch Seldaek/commands (PR #1470)
Commits -------d675c28
[FrameworkBundle] Use Router instead of RouterInterfaceae7ae8d
[FrameworkBundle] Moved router_listener from web to router.xml since it depends on the router35a9023
[FrameworkBundle] Added isEnabled to Router commands, fixes #1467536d979
[Console] Added Command::isEnabled method that defines whether to add the command or not Discussion ---------- [2.1] [Console] Added Command::isEnabled method This addresses #1467. The idea is to allow commands to evaluate whether they can run or not, since they are automatically registered. - It's useful for the two router:* commands since they're optional (router can be disabled), but part of the FrameworkBundle that is not really optional. - It could be useful for third party code as well. - It's BC. - aa95bb0d395810b29a3e654673e130736d9d1080 should address the issue in #1467, while the other commits just make sure the command is not registered at all if the router isn't standard. One issue remains though: - A few other services like twig helpers get the `ròuter` injected, this means that if there is really **no** router service defined, there is still an error. I'm not sure how to fix those beyond adding `on-invalid="null"` but I'm not sure if that's desirable. I guess we could argue that the router is a big candidate for replacement/suppression, and as such it should be truly optional, but if we do it I don't know where it'll lead. I don't want to end up in a situation where half the dependencies are optional to support every possible combination. @fabpot wdyt? --------------------------------------------------------------------------- by kriswallsmith at 2011/06/28 16:19:46 -0700 I'd rather see us not register a command instead of register and then disable it. Can we do the same thing you've done here in the bundle's registerCommands() method? --------------------------------------------------------------------------- by Seldaek at 2011/06/28 16:51:36 -0700 Note that it's never really registered. During the registration it's checked and skipped if not enabled. However, doing it as you suggest means overriding/copy-pasting all the code from the core Bundle class, which I don't like so much. It also means adding code specific to those two commands in a somewhat unrelated place, which I also don't like. I'm not saying the current solution is perfect, but from the alternatives I considered, it's the best I have found. --------------------------------------------------------------------------- by stof at 2011/09/04 04:58:04 -0700 @Seldaek your branch conflicts with master. could you rebase it ? @fabpot what do you think about this PR ? --------------------------------------------------------------------------- by Seldaek at 2011/09/04 08:39:05 -0700 Rebased
This commit is contained in:
commit
063e6f9ae6
@ -17,6 +17,7 @@ use Symfony\Component\Console\Input\InputInterface;
|
||||
use Symfony\Component\Console\Output\OutputInterface;
|
||||
use Symfony\Component\Console\Output\Output;
|
||||
use Symfony\Component\Routing\Matcher\Dumper\ApacheMatcherDumper;
|
||||
use Symfony\Component\Routing\Router;
|
||||
|
||||
/**
|
||||
* RouterApacheDumperCommand.
|
||||
@ -25,6 +26,21 @@ use Symfony\Component\Routing\Matcher\Dumper\ApacheMatcherDumper;
|
||||
*/
|
||||
class RouterApacheDumperCommand extends ContainerAwareCommand
|
||||
{
|
||||
/**
|
||||
* {@inheritDoc}
|
||||
*/
|
||||
public function isEnabled()
|
||||
{
|
||||
if (!$this->getContainer()->has('router')) {
|
||||
return false;
|
||||
}
|
||||
$router = $this->getContainer()->get('router');
|
||||
if (!$router instanceof Router) {
|
||||
return false;
|
||||
}
|
||||
return parent::isEnabled();
|
||||
}
|
||||
|
||||
/**
|
||||
* @see Command
|
||||
*/
|
||||
|
@ -17,6 +17,7 @@ use Symfony\Component\Console\Input\InputInterface;
|
||||
use Symfony\Component\Console\Output\OutputInterface;
|
||||
use Symfony\Component\Console\Output\Output;
|
||||
use Symfony\Component\Routing\Matcher\Dumper\ApacheMatcherDumper;
|
||||
use Symfony\Component\Routing\Router;
|
||||
|
||||
/**
|
||||
* A console command for retrieving information about routes
|
||||
@ -25,6 +26,21 @@ use Symfony\Component\Routing\Matcher\Dumper\ApacheMatcherDumper;
|
||||
*/
|
||||
class RouterDebugCommand extends ContainerAwareCommand
|
||||
{
|
||||
/**
|
||||
* {@inheritDoc}
|
||||
*/
|
||||
public function isEnabled()
|
||||
{
|
||||
if (!$this->getContainer()->has('router')) {
|
||||
return false;
|
||||
}
|
||||
$router = $this->getContainer()->get('router');
|
||||
if (!$router instanceof Router) {
|
||||
return false;
|
||||
}
|
||||
return parent::isEnabled();
|
||||
}
|
||||
|
||||
/**
|
||||
* @see Command
|
||||
*/
|
||||
|
@ -69,5 +69,15 @@
|
||||
<tag name="kernel.cache_warmer" />
|
||||
<argument type="service" id="router" />
|
||||
</service>
|
||||
|
||||
<service id="router_listener" class="%router_listener.class%">
|
||||
<tag name="kernel.event_listener" event="kernel.request" method="onEarlyKernelRequest" priority="255" />
|
||||
<tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" />
|
||||
<tag name="monolog.logger" channel="request" />
|
||||
<argument type="service" id="router" />
|
||||
<argument>%request_listener.http_port%</argument>
|
||||
<argument>%request_listener.https_port%</argument>
|
||||
<argument type="service" id="logger" on-invalid="ignore" />
|
||||
</service>
|
||||
</services>
|
||||
</container>
|
||||
|
@ -24,16 +24,6 @@
|
||||
<argument type="service" id="logger" on-invalid="ignore" />
|
||||
</service>
|
||||
|
||||
<service id="router_listener" class="%router_listener.class%">
|
||||
<tag name="kernel.event_listener" event="kernel.request" method="onEarlyKernelRequest" priority="255" />
|
||||
<tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" />
|
||||
<tag name="monolog.logger" channel="request" />
|
||||
<argument type="service" id="router" />
|
||||
<argument>%request_listener.http_port%</argument>
|
||||
<argument>%request_listener.https_port%</argument>
|
||||
<argument type="service" id="logger" on-invalid="ignore" />
|
||||
</service>
|
||||
|
||||
<service id="response_listener" class="%response_listener.class%">
|
||||
<tag name="kernel.event_listener" event="kernel.response" method="onKernelResponse" />
|
||||
<argument>%kernel.charset%</argument>
|
||||
|
@ -388,6 +388,11 @@ class Application
|
||||
{
|
||||
$command->setApplication($this);
|
||||
|
||||
if (!$command->isEnabled()) {
|
||||
$command->setApplication(null);
|
||||
return;
|
||||
}
|
||||
|
||||
$this->commands[$command->getName()] = $command;
|
||||
|
||||
foreach ($command->getAliases() as $alias) {
|
||||
|
@ -116,6 +116,19 @@ class Command
|
||||
return $this->application;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether the command is enabled or not in the current environment
|
||||
*
|
||||
* Override this to check for x or y and return false if the command can not
|
||||
* run properly under the current conditions.
|
||||
*
|
||||
* @return Boolean
|
||||
*/
|
||||
public function isEnabled()
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Configures the current command.
|
||||
*/
|
||||
|
Reference in New Issue
Block a user