Kdyby / Console

Symfony Console integration for Kdyby components
https://packagist.org/packages/kdyby/console
Other
52 stars 54 forks source link

Bugfix: anonymous commands can't have same argument sequence #45

Closed foglcz closed 8 years ago

foglcz commented 8 years ago
Type Bugfix
Fixes issues n/a - not reported
Documentation not needed
BC Break no
Tests updated yes

When we register commands as anonymous services with parameters, they get registered in container with name "command(md5hash)". The MD5 is constructed from json_encode, which encodes instance of Nette\DI\Statement.

If we have following .neon file configuration (real world example):

console:
    commands:
        - App\Console\SynchronizeToOI(@shopDibi, @oiDibi)
        - App\Console\SynchronizeToShops(@shopDibi, @oiDibi)

The second command fails to register with message Service 'console.command.b84a00605660d460bba12d52a02bf1d8' has already been added.

This is because the md5 hash actually hashes only public properties of the Statement - which does not contain the class name.

Test code in ConsoleExtension.php:78

        foreach ($config['commands'] as $command) {
            echo Nette\Utils\Json::encode($command) . " ---> ";
            echo md5(Nette\Utils\Json::encode($command)) . "\n";
        }

Test output

C:\Users\pptacek\www\projectname>php www/index.php
{"arguments":["@shopDibi","@oiDibi"]} ---> b84a00605660d460bba12d52a02bf1d8
{"arguments":["@shopDibi","@oiDibi"]} ---> b84a00605660d460bba12d52a02bf1d8

Effectively, all anonymous commands with constructor-inject, were hashed solely based on their parameter sequence.

Fixed debug code:

        foreach ($config['commands'] as $command) {
            $entityName = ($command instanceof Statement) ? $command->getEntity() : '';
            echo $entityName.Nette\Utils\Json::encode($command) . " ---> ";
            echo md5($entityName.Nette\Utils\Json::encode($command)) . "\n";
        }
        exit;

Fixed debug output:

C:\Users\pptacek\www\projectname>php www/index.php
App\Console\SynchronizeToOI{"arguments":["@shopDibi","@oiDibi"]} ---> c2af7ff37cb79a56f83aaf2a40874694
App\Console\SynchronizeToShops{"arguments":["@shopDibi","@oiDibi"]} ---> 2636b2fdc12987674438dd205cf53208

Implementation note the md5 hash can be fixed by using var_export or print_r($entity, true) instead of JSON. Since JSON has been selected originally, I tried to stick with something compact & human-readable (thus prepending the name if working with Statement).

fprochazka commented 8 years ago

@foglcz thank you very much! And amazing report by the way :)