Issues in ChannelFactory.php - New Issues - Inspection of "Improce configuration" - irishdan/NotificationBundle - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( e7a3c7...2372b5 )

by dan

created 2017-09-03 12:39 UTC

DependencyInjection/Factory/ChannelFactory.php (6 issues)

Labels

Bug 6

Severity

Minor 6

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php

namespace IrishDan\NotificationBundle\DependencyInjection\Factory;

use Symfony\Component\Config\Definition\Builder\NodeDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference;

class ChannelFactory
{
    protected $channelKey;
    protected $adapterName;
    protected $adapterConfiguration;

    public function getChannelKeyAdapterMap()
    {
        return [
            'channel' => $this->channelKey,
            'adapter' => $this->adapterName,
            'config' => $this->adapterConfiguration,
        ];
    }

    public function create(ContainerBuilder $container, $channel, array $config, $type = '')
    {
        if (empty($type)) {
            $type = $channel;
        }

        // Merge config with type config
        if ($container->hasParameter('notification.channel.' . $type . '.configuration')) {
            $defaultConfig = $container->getParameter('notification.channel.' . $type . '.configuration');
            $config = array_merge($defaultConfig, $config);
        }

        $adapterName = 'notification.adapter.' . $type;
        $adapter = new Reference($adapterName);
        $eventDispatcher = new Reference('event_dispatcher');

        $parameterName = 'notification.channel.' . $channel . '.configuration';
        if (!$container->hasParameter($parameterName)) {
            $container->setParameter($parameterName, $config);
        }

        $definition = new Definition();
        $definition->setClass('IrishDan\NotificationBundle\Channel\DirectChannel');
        $definition->setArguments(
            [
                '%' . $parameterName . '%',
                $channel,
                $adapter,
            ]
        );

        $definition->setMethodCalls(
            [
                ['setEventDispatcher', [$eventDispatcher]],
            ]
        );

        $definition->addMethodCall('setDispatchToEvent', [empty($config['direct_dispatch'])]);

        $serviceName = 'notification.channel.' . $channel;
        $container->setDefinition($serviceName, $definition);

        $this->adapterConfiguration = $parameterName;
        $this->adapterName = $adapterName;
        $this->channelKey = $type;

        return $serviceName;
    }

    public function addConfiguration(NodeDefinition $node, $type)
    {
        switch ($type) {
            case 'mail':
                $node
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
                    ->children()
                    ->scalarNode('default_sender')->defaultValue('')->end()
                    ->arrayNode('cc')->end()
                    ->arrayNode('bcc')->end()
                    ->end();
                break;

            case 'database':
                $node
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
                    ->children()
                    ->scalarNode('entity')->defaultValue('AppBundle:Notification')->end()
                    ->end();
                break;
            case 'pusher':
                $node
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
                    ->children()
                    ->scalarNode('auth_key')->defaultValue('')->end()
                    ->scalarNode('secret')->defaultValue('')->end()
                    ->scalarNode('app_id')->defaultValue('')->end()
                    ->scalarNode('cluster')->defaultValue('')->end()
                    ->booleanNode('encrypted')->defaultTrue()->end()
                    ->scalarNode('channel_name')->defaultValue('')->end()
                    ->scalarNode('event')->defaultValue('')->end()
                    ->end();
                break;
            case 'nexmo':
                $node
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
                    ->children()
                    ->scalarNode('api_key')->defaultValue('')->end()
                    ->scalarNode('api_secret')->defaultValue('')->end()
                    ->scalarNode('from')->defaultValue('')->end()
                    ->end();
                break;
            case 'slack':
                $node
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
                    ->children()
                    ->scalarNode('webhook')->defaultNull()->end()
                    ->end();
                break;
            case 'logger':
                $node
abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
                    ->children()
                    ->scalarNode('type')->defaultValue('info')->end()
                    ->end();
                break;
            default:
                // Should allow any config
                // @TODO: The type key to define the allowed configuration
                break;
        }
    }
}


1			<?php
2
3			namespace IrishDan\NotificationBundle\DependencyInjection\Factory;
4
5			use Symfony\Component\Config\Definition\Builder\NodeDefinition;
6			use Symfony\Component\DependencyInjection\ContainerBuilder;
7			use Symfony\Component\DependencyInjection\Definition;
8			use Symfony\Component\DependencyInjection\Reference;
9
10			class ChannelFactory
11			{
12			protected $channelKey;
13			protected $adapterName;
14			protected $adapterConfiguration;
15
16			public function getChannelKeyAdapterMap()
17			{
18			return [
19			'channel' => $this->channelKey,
20			'adapter' => $this->adapterName,
21			'config' => $this->adapterConfiguration,
22			];
23			}
24
25			public function create(ContainerBuilder $container, $channel, array $config, $type = '')
26			{
27			if (empty($type)) {
28			$type = $channel;
29			}
30
31			// Merge config with type config
32			if ($container->hasParameter('notification.channel.' . $type . '.configuration')) {
33			$defaultConfig = $container->getParameter('notification.channel.' . $type . '.configuration');
34			$config = array_merge($defaultConfig, $config);
35			}
36
37			$adapterName = 'notification.adapter.' . $type;
38			$adapter = new Reference($adapterName);
39			$eventDispatcher = new Reference('event_dispatcher');
40
41			$parameterName = 'notification.channel.' . $channel . '.configuration';
42			if (!$container->hasParameter($parameterName)) {
43			$container->setParameter($parameterName, $config);
44			}
45
46			$definition = new Definition();
47			$definition->setClass('IrishDan\NotificationBundle\Channel\DirectChannel');
48			$definition->setArguments(
49			[
50			'%' . $parameterName . '%',
51			$channel,
52			$adapter,
53			]
54			);
55
56			$definition->setMethodCalls(
57			[
58			['setEventDispatcher', [$eventDispatcher]],
59			]
60			);
61
62			$definition->addMethodCall('setDispatchToEvent', [empty($config['direct_dispatch'])]);
63
64			$serviceName = 'notification.channel.' . $channel;
65			$container->setDefinition($serviceName, $definition);
66
67			$this->adapterConfiguration = $parameterName;
68			$this->adapterName = $adapterName;
69			$this->channelKey = $type;
70
71			return $serviceName;
72			}
73
74			public function addConfiguration(NodeDefinition $node, $type)
75			{
76			switch ($type) {
77			case 'mail':
78			$node
			0 ignored issues – show Bug introduced 2017-09-03 12:41 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a specific sub-type and not the parent class `Symfony\Component\Config...\Builder\NodeDefinition` as the method `children()` does only exist in the following sub-classes of `Symfony\Component\Config...\Builder\NodeDefinition`: `Symfony\Component\Config...der\ArrayNodeDefinition`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
79			->children()
80			->scalarNode('default_sender')->defaultValue('')->end()
81			->arrayNode('cc')->end()
82			->arrayNode('bcc')->end()
83			->end();
84			break;
85
86			case 'database':
87			$node
			0 ignored issues – show Bug introduced 2017-09-03 12:41 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a specific sub-type and not the parent class `Symfony\Component\Config...\Builder\NodeDefinition` as the method `children()` does only exist in the following sub-classes of `Symfony\Component\Config...\Builder\NodeDefinition`: `Symfony\Component\Config...der\ArrayNodeDefinition`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
88			->children()
89			->scalarNode('entity')->defaultValue('AppBundle:Notification')->end()
90			->end();
91			break;
92			case 'pusher':
93			$node
			0 ignored issues – show Bug introduced 2017-09-03 12:41 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a specific sub-type and not the parent class `Symfony\Component\Config...\Builder\NodeDefinition` as the method `children()` does only exist in the following sub-classes of `Symfony\Component\Config...\Builder\NodeDefinition`: `Symfony\Component\Config...der\ArrayNodeDefinition`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
94			->children()
95			->scalarNode('auth_key')->defaultValue('')->end()
96			->scalarNode('secret')->defaultValue('')->end()
97			->scalarNode('app_id')->defaultValue('')->end()
98			->scalarNode('cluster')->defaultValue('')->end()
99			->booleanNode('encrypted')->defaultTrue()->end()
100			->scalarNode('channel_name')->defaultValue('')->end()
101			->scalarNode('event')->defaultValue('')->end()
102			->end();
103			break;
104			case 'nexmo':
105			$node
			0 ignored issues – show Bug introduced 2017-09-03 12:41 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a specific sub-type and not the parent class `Symfony\Component\Config...\Builder\NodeDefinition` as the method `children()` does only exist in the following sub-classes of `Symfony\Component\Config...\Builder\NodeDefinition`: `Symfony\Component\Config...der\ArrayNodeDefinition`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
106			->children()
107			->scalarNode('api_key')->defaultValue('')->end()
108			->scalarNode('api_secret')->defaultValue('')->end()
109			->scalarNode('from')->defaultValue('')->end()
110			->end();
111			break;
112			case 'slack':
113			$node
			0 ignored issues – show Bug introduced 2017-09-03 12:41 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a specific sub-type and not the parent class `Symfony\Component\Config...\Builder\NodeDefinition` as the method `children()` does only exist in the following sub-classes of `Symfony\Component\Config...\Builder\NodeDefinition`: `Symfony\Component\Config...der\ArrayNodeDefinition`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
114			->children()
115			->scalarNode('webhook')->defaultNull()->end()
116			->end();
117			break;
118			case 'logger':
119			$node
			0 ignored issues – show Bug introduced 2017-09-03 12:41 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like you code against a specific sub-type and not the parent class `Symfony\Component\Config...\Builder\NodeDefinition` as the method `children()` does only exist in the following sub-classes of `Symfony\Component\Config...\Builder\NodeDefinition`: `Symfony\Component\Config...der\ArrayNodeDefinition`. Maybe you want to `instanceof` check for one of these explicitly? Let’s take a look at an example: abstract class User { /** @return string / abstract public function getPassword(); } class MyUser extends User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the parent class: abstract class User { /** @return string / abstract public function getPassword(); /* @return string */ abstract public function getDisplayName(); } Loading history...
120			->children()
121			->scalarNode('type')->defaultValue('info')->end()
122			->end();
123			break;
124			default:
125			// Should allow any config
126			// @TODO: The type key to define the allowed configuration
127			break;
128			}
129			}
130			}
131

irishdan / NotificationBundle

Push — master ( e7a3c7...2372b5 )

DependencyInjection/Factory/ChannelFactory.php (6 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like